From fe63e9f48664163e39d32167aad33a9d47e91203 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:08:54 -0400 Subject: [PATCH 01/15] fix(execution): reject invalid costs atomically Validate configured and custom execution costs before any fill or financial state mutation, including flip commissions and final execution prices. Closes #41. --- .pre-commit-config.yaml | 2 +- src/ml4t/backtest/accounting/gatekeeper.py | 8 +- src/ml4t/backtest/broker.py | 2 + src/ml4t/backtest/config.py | 47 +++- src/ml4t/backtest/core/execution_engine.py | 13 +- src/ml4t/backtest/core/order_book.py | 21 +- src/ml4t/backtest/datafeed.py | 1 - src/ml4t/backtest/execution/fill_executor.py | 90 ++++++-- src/ml4t/backtest/execution/schedule.py | 1 - src/ml4t/backtest/models.py | 39 ++++ src/ml4t/backtest/result.py | 1 - tests/contracts/test_execution_contracts.py | 2 +- tests/execution/test_rebalancer.py | 2 +- tests/execution/test_schedule.py | 2 +- tests/test_artifact_spec.py | 7 +- tests/test_broker.py | 2 +- tests/test_config_wiring.py | 2 +- tests/test_core.py | 2 +- tests/test_cost_validation.py | 228 +++++++++++++++++++ tests/test_datafeed_memory.py | 2 +- tests/test_equity_curve.py | 2 +- tests/test_result.py | 2 +- tests/test_strategy_templates.py | 2 +- 23 files changed, 423 insertions(+), 57 deletions(-) create mode 100644 tests/test_cost_validation.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c965d5d5..e275c6d0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.12.10 + rev: v0.14.10 hooks: - id: ruff-format - id: ruff diff --git a/src/ml4t/backtest/accounting/gatekeeper.py b/src/ml4t/backtest/accounting/gatekeeper.py index ab1696f7..9d68ac1f 100644 --- a/src/ml4t/backtest/accounting/gatekeeper.py +++ b/src/ml4t/backtest/accounting/gatekeeper.py @@ -6,7 +6,7 @@ from collections.abc import Callable -from ..models import CommissionModel +from ..models import CommissionModel, calculate_commission from ..types import Order, OrderSide from .account import AccountState @@ -137,7 +137,9 @@ def validate_order(self, order: Order, price: float) -> tuple[bool, str]: # Check for position reversal (long→short or short→long) # Delegate to policy's handle_reversal() method if self._is_reversal(current_qty, order_qty_delta): - commission = self.commission_model.calculate(order.asset, order.quantity, price) + commission = calculate_commission( + self.commission_model, order.asset, order.quantity, price + ) return self.account.policy.handle_reversal( asset=order.asset, current_quantity=current_qty, @@ -156,7 +158,7 @@ def validate_order(self, order: Order, price: float) -> tuple[bool, str]: # This is an opening order (new position or adding to existing) # Calculate commission to include in cost - commission = self.commission_model.calculate(order.asset, order.quantity, price) + commission = calculate_commission(self.commission_model, order.asset, order.quantity, price) # Use buffered cash (reserves cash_buffer_pct for safety margin) available = self._available_cash() diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 6db6ebf0..1fdcccb6 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -298,6 +298,8 @@ def from_config( VolumeShareSlippage, ) + config._validate_for_execution() + effective_commission_type = config.commission_type if effective_commission_type == CommissionType.NONE: if config.commission_per_share > 0: diff --git a/src/ml4t/backtest/config.py b/src/ml4t/backtest/config.py index c6aa3dc2..2b26c61e 100644 --- a/src/ml4t/backtest/config.py +++ b/src/ml4t/backtest/config.py @@ -22,6 +22,7 @@ from __future__ import annotations +import math import os from dataclasses import asdict, dataclass, field, replace from enum import Enum @@ -29,7 +30,6 @@ from typing import Any import yaml - from ml4t.specs.base import serialize_artifact_value from ml4t.specs.market_data import FeedSpec, TimestampSemantics @@ -535,7 +535,7 @@ def validate(self, warn: bool = True) -> list[str]: """ import warnings as _warnings - issues: list[str] = [] + issues: list[str] = self._execution_validation_errors() # Look-ahead bias warning if self.execution_mode == ExecutionMode.SAME_BAR: @@ -565,12 +565,6 @@ def validate(self, warn: bool = True) -> list[str]: "Verify this matches your broker's actual costs." ) - if self.slippage_spread < 0: - issues.append(f"slippage_spread ({self.slippage_spread}) must be >= 0") - - if any(spread < 0 for spread in self.slippage_spread_by_asset.values()): - issues.append("slippage_spread_by_asset values must all be >= 0") - if ( self.slippage_type == SlippageType.SPREAD and self.slippage_spread == 0.0 @@ -641,6 +635,43 @@ def validate(self, warn: bool = True) -> list[str]: return issues + def _execution_validation_errors(self) -> list[str]: + errors: list[str] = [] + cost_fields = ( + "commission_rate", + "commission_per_share", + "commission_per_trade", + "commission_minimum", + "slippage_rate", + "slippage_fixed", + "slippage_spread", + "stop_slippage_rate", + ) + for field_name in cost_fields: + value = getattr(self, field_name) + try: + valid = math.isfinite(value) and value >= 0.0 + except TypeError: + valid = False + if not valid: + errors.append(f"{field_name} ({value!r}) must be finite and >= 0") + + for asset, spread in self.slippage_spread_by_asset.items(): + try: + valid = math.isfinite(spread) and spread >= 0.0 + except TypeError: + valid = False + if not valid: + errors.append( + f"slippage_spread_by_asset[{asset!r}] ({spread!r}) must be finite and >= 0" + ) + return errors + + def _validate_for_execution(self) -> None: + errors = self._execution_validation_errors() + if errors: + raise ValueError("Invalid BacktestConfig: " + "; ".join(errors)) + def get_effective_account_settings(self) -> tuple[bool, bool]: """Get account settings as a tuple. diff --git a/src/ml4t/backtest/core/execution_engine.py b/src/ml4t/backtest/core/execution_engine.py index b612bb6e..86acf3f8 100644 --- a/src/ml4t/backtest/core/execution_engine.py +++ b/src/ml4t/backtest/core/execution_engine.py @@ -4,6 +4,7 @@ import copy +from ..models import calculate_commission from ..types import ExecutionMode, OrderSide, OrderStatus, OrderType, Position from .shared import is_exit_order @@ -239,8 +240,8 @@ def _validate_shadow_queue_order( and abs(new_qty) > 1e-12 and ((current_qty > 0 and new_qty < 0) or (current_qty < 0 and new_qty > 0)) ) - commission = broker.commission_model.calculate( - order.asset, order.quantity, validation_price + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, validation_price ) multiplier = broker.get_multiplier(order.asset) @@ -288,7 +289,9 @@ def _commit_shadow_queue_fill( shadow_positions[order.asset].quantity if order.asset in shadow_positions else 0.0 ) new_qty = current_qty + qty_delta - commission = broker.commission_model.calculate(order.asset, order.quantity, fill_price) + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, fill_price + ) shadow_cash += -qty_delta * fill_price * broker.get_multiplier(order.asset) - commission if abs(new_qty) <= 1e-12: @@ -586,7 +589,9 @@ def _passes_simple_cash_check(self, order, fill_price: float) -> bool: return True signed_qty = order.quantity if order.side is OrderSide.BUY else -order.quantity - commission = broker.commission_model.calculate(order.asset, order.quantity, fill_price) + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, fill_price + ) projected_cash = broker.cash - signed_qty * fill_price - commission return projected_cash >= 0.0 diff --git a/src/ml4t/backtest/core/order_book.py b/src/ml4t/backtest/core/order_book.py index 8ff7dcb5..e0ecfe68 100644 --- a/src/ml4t/backtest/core/order_book.py +++ b/src/ml4t/backtest/core/order_book.py @@ -4,6 +4,7 @@ from datetime import datetime +from ..models import calculate_commission from ..types import ExecutionMode, Order, OrderSide, OrderStatus, OrderType, Position from .shared import SubmitOrderOptions, is_exit_order @@ -339,16 +340,16 @@ def _passes_submission_precheck(self, order: Order) -> bool: if closed != 0.0: close_cash = (-closed) * signal_price shadow_cash += close_cash - closed_commission = broker.commission_model.calculate( - order.asset, abs(closed), signal_price + closed_commission = calculate_commission( + broker.commission_model, order.asset, abs(closed), signal_price ) shadow_cash -= closed_commission if opened != 0.0: open_cash = opened * signal_price shadow_cash -= open_cash - opened_commission = broker.commission_model.calculate( - order.asset, abs(opened), signal_price + opened_commission = calculate_commission( + broker.commission_model, order.asset, abs(opened), signal_price ) shadow_cash -= opened_commission @@ -419,8 +420,8 @@ def _passes_buying_power_check(self, order: Order) -> bool: if closed != 0.0: closed_value = (-closed) * signal_price shadow_cash += closed_value - closed_commission = broker.commission_model.calculate( - order.asset, abs(closed), signal_price + closed_commission = calculate_commission( + broker.commission_model, order.asset, abs(closed), signal_price ) shadow_cash -= closed_commission @@ -430,8 +431,8 @@ def _passes_buying_power_check(self, order: Order) -> bool: # This prevents credit-model inflation where short proceeds # artificially inflate shadow cash. shadow_cash -= abs(opened) * signal_price - opened_commission = broker.commission_model.calculate( - order.asset, abs(opened), signal_price + opened_commission = calculate_commission( + broker.commission_model, order.asset, abs(opened), signal_price ) shadow_cash -= opened_commission @@ -461,7 +462,9 @@ def _passes_margin_submission_precheck(self, order: Order, signal_price: float) old_qty, old_price, size, signal_price ) - commission = broker.commission_model.calculate(order.asset, order.quantity, signal_price) + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, signal_price + ) available_cash = self._submission_shadow_cash if broker.cash_buffer_pct > 0 and available_cash > 0: available_cash *= 1.0 - broker.cash_buffer_pct diff --git a/src/ml4t/backtest/datafeed.py b/src/ml4t/backtest/datafeed.py index 75ff7de8..6a7c74cb 100644 --- a/src/ml4t/backtest/datafeed.py +++ b/src/ml4t/backtest/datafeed.py @@ -9,7 +9,6 @@ from typing import Any import polars as pl - from ml4t.specs.market_data import FeedSpec diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index 1818c59d..6b36d42e 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -7,11 +7,13 @@ from __future__ import annotations +import math from dataclasses import dataclass from datetime import datetime from typing import TYPE_CHECKING from ..config import InitialHwmSource, ShareType +from ..models import calculate_commission, calculate_slippage from ..types import ( ExitReason, Fill, @@ -58,6 +60,8 @@ class FillContext: is_partial: bool price_source: str quote_context: dict[str, float | None] + close_commission: float | None = None + open_commission: float | None = None class FillExecutor: @@ -100,6 +104,8 @@ def execute(self, order: Order, base_price: float) -> bool: current_time = broker._current_time assert current_time is not None, "Cannot execute fill without current time" + self._validate_execution_price(base_price, source="base execution price") + available_size = broker.get_available_size(order.asset, order.side) # Get effective quantity (considering partial fills from previous bars) @@ -107,6 +113,7 @@ def execute(self, order: Order, base_price: float) -> bool: fill_quantity = effective_quantity # Apply execution limits (volume participation) + remaining_quantity = 0.0 if broker.execution_limits is not None: if order.order_id in broker._filled_this_bar: return False @@ -121,20 +128,18 @@ def execute(self, order: Order, base_price: float) -> bool: if broker.share_type == ShareType.INTEGER: fill_quantity = float(int(fill_quantity)) - if fill_quantity <= 0: + if not math.isfinite(fill_quantity) or fill_quantity < 0: + raise ValueError( + "Invalid execution quantity from " + f"{type(broker.execution_limits).__name__}: got {fill_quantity!r}" + ) + if fill_quantity == 0: return False - broker._filled_this_bar.add(order.order_id) - remaining_quantity = max(0.0, effective_quantity - fill_quantity) if broker.share_type == ShareType.INTEGER: remaining_quantity = float(int(remaining_quantity)) - if remaining_quantity > 0: - broker._partial_orders[order.order_id] = remaining_quantity - else: - broker._partial_orders.pop(order.order_id, None) - # Apply market impact if broker.market_impact_model is not None: is_buy = order.side == OrderSide.BUY @@ -144,21 +149,55 @@ def execute(self, order: Order, base_price: float) -> bool: available_size, is_buy, ) + self._validate_market_impact(impact, is_buy=is_buy) base_price = base_price + impact + self._validate_execution_price(base_price, source="market-impact execution price") # Calculate slippage - slippage = broker.slippage_model.calculate( + slippage = calculate_slippage( + broker.slippage_model, order.asset, fill_quantity, base_price, available_size, ) fill_price = base_price + slippage if order.side == OrderSide.BUY else base_price - slippage + self._validate_execution_price(fill_price, source="execution price") # Calculate commission - commission = broker.commission_model.calculate(order.asset, fill_quantity, fill_price) + commission = calculate_commission( + broker.commission_model, order.asset, fill_quantity, fill_price + ) quote_context = broker.get_quote_context(order.asset, order.side) + signed_qty = fill_quantity if order.side == OrderSide.BUY else -fill_quantity + close_commission = None + open_commission = None + position = broker.positions.get(order.asset) + if position is not None: + new_qty = position.quantity + signed_qty + is_flip = position.quantity > 0 > new_qty or position.quantity < 0 < new_qty + if is_flip: + close_commission = calculate_commission( + broker.commission_model, + order.asset, + abs(position.quantity), + fill_price, + ) + open_commission = calculate_commission( + broker.commission_model, + order.asset, + abs(new_qty), + fill_price, + ) + + if broker.execution_limits is not None: + broker._filled_this_bar.add(order.order_id) + if remaining_quantity > 0: + broker._partial_orders[order.order_id] = remaining_quantity + else: + broker._partial_orders.pop(order.order_id, None) + # Create fill record fill = Fill( order_id=order.order_id, @@ -196,7 +235,6 @@ def execute(self, order: Order, base_price: float) -> bool: order.filled_quantity = fill_quantity # Build fill context - signed_qty = fill_quantity if order.side == OrderSide.BUY else -fill_quantity ctx = FillContext( order=order, current_time=current_time, @@ -208,6 +246,8 @@ def execute(self, order: Order, base_price: float) -> bool: is_partial=is_partial, price_source=broker.execution_price.value, quote_context=quote_context, + close_commission=close_commission, + open_commission=open_commission, ) # Update position and get actual commission (may change for flips) @@ -241,6 +281,25 @@ def execute(self, order: Order, base_price: float) -> bool: return not is_partial + @staticmethod + def _validate_execution_price(value: float, *, source: str) -> None: + if not math.isfinite(value) or value <= 0.0: + raise ValueError(f"Invalid {source}: expected a finite positive number, got {value!r}") + + def _validate_market_impact(self, value: float, *, is_buy: bool) -> None: + model_name = type(self.broker.market_impact_model).__name__ + if not math.isfinite(value): + raise ValueError( + f"Invalid market impact from {model_name}: expected a finite adverse value, " + f"got {value!r}" + ) + wrong_direction = (is_buy and value < 0.0) or (not is_buy and value > 0.0) + if wrong_direction: + expected = ">= 0 for buys" if is_buy else "<= 0 for sells" + raise ValueError( + f"Invalid market impact from {model_name}: expected {expected}, got {value!r}" + ) + def _update_position(self, ctx: FillContext) -> float: """Update position based on fill. @@ -439,12 +498,11 @@ def _flip_position( broker = self.broker order = ctx.order - close_qty = abs(old_qty) - open_qty = abs(new_qty) - # Calculate separate commissions for close and open portions - close_commission = broker.commission_model.calculate(order.asset, close_qty, ctx.fill_price) - open_commission = broker.commission_model.calculate(order.asset, open_qty, ctx.fill_price) + assert ctx.close_commission is not None + assert ctx.open_commission is not None + close_commission = ctx.close_commission + open_commission = ctx.open_commission total_commission = close_commission + open_commission # Close the old position (include multiplier for futures) diff --git a/src/ml4t/backtest/execution/schedule.py b/src/ml4t/backtest/execution/schedule.py index 9f713537..a9b4f463 100644 --- a/src/ml4t/backtest/execution/schedule.py +++ b/src/ml4t/backtest/execution/schedule.py @@ -9,7 +9,6 @@ from typing import Any import polars as pl - from ml4t.specs.market_data import FeedSpec, TimestampSemantics from ..calendar import get_schedule diff --git a/src/ml4t/backtest/models.py b/src/ml4t/backtest/models.py index e7b7a0bc..ed8aeb4b 100644 --- a/src/ml4t/backtest/models.py +++ b/src/ml4t/backtest/models.py @@ -1,5 +1,6 @@ """Pluggable commission and slippage models.""" +import math from typing import Protocol, runtime_checkable # === Protocols === @@ -21,6 +22,44 @@ def calculate( ) -> float: ... +def calculate_commission( + model: CommissionModel, + asset: str, + quantity: float, + price: float, +) -> float: + value = model.calculate(asset, quantity, price) + return _validate_nonnegative_model_output("commission", model, value) + + +def calculate_slippage( + model: SlippageModel, + asset: str, + quantity: float, + price: float, + volume: float | None, +) -> float: + value = model.calculate(asset, quantity, price, volume) + return _validate_nonnegative_model_output("slippage", model, value) + + +def _validate_nonnegative_model_output(kind: str, model: object, value: float) -> float: + model_name = type(model).__name__ + try: + numeric_value = float(value) + except (TypeError, ValueError) as exc: + raise ValueError( + f"Invalid {kind} from {model_name}: expected a finite non-negative number, " + f"got {value!r}" + ) from exc + if not math.isfinite(numeric_value) or numeric_value < 0.0: + raise ValueError( + f"Invalid {kind} from {model_name}: expected a finite non-negative number, " + f"got {value!r}" + ) + return numeric_value + + # === Commission Models === diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 438a22ad..34f70419 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -28,7 +28,6 @@ from typing import TYPE_CHECKING, Any, Literal import polars as pl - from ml4t.specs.market_data import FeedSpec try: diff --git a/tests/contracts/test_execution_contracts.py b/tests/contracts/test_execution_contracts.py index 4819131c..e6e4dc3a 100644 --- a/tests/contracts/test_execution_contracts.py +++ b/tests/contracts/test_execution_contracts.py @@ -4,6 +4,7 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.config import ( BacktestConfig, @@ -15,7 +16,6 @@ from ml4t.backtest.engine import run_backtest from ml4t.backtest.strategy import Strategy from ml4t.backtest.types import ExecutionMode -from ml4t.specs.market_data import FeedSpec def _prices() -> pl.DataFrame: diff --git a/tests/execution/test_rebalancer.py b/tests/execution/test_rebalancer.py index 0359dda5..e58ef131 100644 --- a/tests/execution/test_rebalancer.py +++ b/tests/execution/test_rebalancer.py @@ -3,6 +3,7 @@ from datetime import datetime import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import ( Broker, @@ -12,7 +13,6 @@ from ml4t.backtest.execution.rebalancer import RebalanceConfig, TargetWeightExecutor from ml4t.backtest.execution.schedule import RebalanceSchedule from ml4t.backtest.models import NoCommission, NoSlippage -from ml4t.specs.market_data import FeedSpec class TestRebalanceConfig: diff --git a/tests/execution/test_schedule.py b/tests/execution/test_schedule.py index 9a578169..00bca0aa 100644 --- a/tests/execution/test_schedule.py +++ b/tests/execution/test_schedule.py @@ -5,13 +5,13 @@ from datetime import UTC, datetime import polars as pl +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.execution import ( RebalanceCadence, RebalanceSchedule, resolve_rebalance_timestamps, ) -from ml4t.specs.market_data import FeedSpec def _make_weekday_series(start: str, end: str) -> pl.Series: diff --git a/tests/test_artifact_spec.py b/tests/test_artifact_spec.py index c6a3f093..a37f7d25 100644 --- a/tests/test_artifact_spec.py +++ b/tests/test_artifact_spec.py @@ -2,13 +2,14 @@ from pathlib import Path +from ml4t.diagnostic.artifacts import dump_spec, load_market_data_spec, load_spec +from ml4t.engineer.artifacts import FeatureSpec, LabelSpec, PredictionSpec +from ml4t.specs import ArtifactKind, FeedSpec, MarketDataSpec, TimestampSemantics + from ml4t.backtest.spec_bridge import ( market_data_spec_to_feed_spec, market_data_spec_to_runtime_metadata, ) -from ml4t.diagnostic.artifacts import dump_spec, load_market_data_spec, load_spec -from ml4t.engineer.artifacts import FeatureSpec, LabelSpec, PredictionSpec -from ml4t.specs import ArtifactKind, FeedSpec, MarketDataSpec, TimestampSemantics def test_market_data_spec_from_mapping_normalizes_timestamp_semantics() -> None: diff --git a/tests/test_broker.py b/tests/test_broker.py index e6f660f1..735229fd 100644 --- a/tests/test_broker.py +++ b/tests/test_broker.py @@ -3,6 +3,7 @@ from datetime import datetime import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.broker import Broker from ml4t.backtest.config import ShareType @@ -16,7 +17,6 @@ OrderType, Position, ) -from ml4t.specs.market_data import FeedSpec @pytest.fixture diff --git a/tests/test_config_wiring.py b/tests/test_config_wiring.py index 041a2b26..2736fb96 100644 --- a/tests/test_config_wiring.py +++ b/tests/test_config_wiring.py @@ -12,6 +12,7 @@ from datetime import datetime import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import ( BacktestConfig, @@ -41,7 +42,6 @@ VolumeShareSlippage, ) from ml4t.backtest.types import OrderSide, Position -from ml4t.specs.market_data import FeedSpec # --------------------------------------------------------------------------- # Helpers diff --git a/tests/test_core.py b/tests/test_core.py index 85cdabdb..80fdac61 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -4,6 +4,7 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import ( Broker, @@ -23,7 +24,6 @@ SlippageType, ) from ml4t.backtest.models import PercentageCommission, VolumeShareSlippage -from ml4t.specs.market_data import FeedSpec # === Test Data Generators === diff --git a/tests/test_cost_validation.py b/tests/test_cost_validation.py new file mode 100644 index 00000000..4ab00d4c --- /dev/null +++ b/tests/test_cost_validation.py @@ -0,0 +1,228 @@ +from __future__ import annotations + +import copy +import math +from datetime import datetime, timedelta +from typing import Any + +import polars as pl +import pytest + +from ml4t.backtest import BacktestConfig, DataFeed, Engine, ExecutionMode, Strategy, run_backtest + + +class _NoOpStrategy(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + return None + + +class _BuyOnceStrategy(Strategy): + def __init__(self) -> None: + self.submitted = False + self.broker_snapshot: dict[str, Any] | None = None + + def on_data(self, timestamp, data, context, broker) -> None: + if self.submitted: + return + broker.submit_order("AAPL", 10) + self.submitted = True + self.broker_snapshot = _financial_snapshot(broker) + + +class _RoundTripStrategy(Strategy): + def __init__(self) -> None: + self.entered = False + self.exit_submitted = False + self.broker_snapshot: dict[str, Any] | None = None + + def on_data(self, timestamp, data, context, broker) -> None: + position = broker.get_position("AAPL") + if not self.entered: + broker.submit_order("AAPL", 10) + self.entered = True + elif position is not None and not self.exit_submitted: + broker.close_position("AAPL") + self.exit_submitted = True + self.broker_snapshot = _financial_snapshot(broker) + + +class _ConstantCommission: + def __init__(self, value: float) -> None: + self.value = value + + def calculate(self, asset: str, quantity: float, price: float) -> float: + return self.value + + +class _ConstantSlippage: + def __init__(self, value: float) -> None: + self.value = value + + def calculate( + self, + asset: str, + quantity: float, + price: float, + volume: float | None, + ) -> float: + return self.value + + +class _ConstantImpact: + def __init__(self, value: float) -> None: + self.value = value + + def calculate( + self, + quantity: float, + price: float, + volume: float | None, + is_buy: bool, + ) -> float: + return self.value + + +class _InvalidSellPriceImpact: + def __init__(self) -> None: + self.broker = None + self.broker_snapshot: dict[str, Any] | None = None + + def calculate( + self, + quantity: float, + price: float, + volume: float | None, + is_buy: bool, + ) -> float: + if not is_buy: + self.broker_snapshot = _financial_snapshot(self.broker) + return 0.0 if is_buy else -200.0 + + +def _prices() -> pl.DataFrame: + start = datetime(2024, 1, 2) + timestamps = [start + timedelta(days=offset) for offset in range(4)] + return pl.DataFrame( + { + "timestamp": timestamps, + "symbol": ["AAPL"] * len(timestamps), + "open": [100.0] * len(timestamps), + "high": [100.0] * len(timestamps), + "low": [100.0] * len(timestamps), + "close": [100.0] * len(timestamps), + "volume": [1_000_000.0] * len(timestamps), + } + ) + + +def _financial_snapshot(broker) -> dict[str, Any]: + return { + "cash": broker.cash, + "account_cash": broker.account.cash, + "positions": copy.deepcopy(broker.positions), + "account_positions": copy.deepcopy(broker.account.positions), + "orders": copy.deepcopy(broker.orders), + "pending_orders": copy.deepcopy(broker.pending_orders), + "fills": copy.deepcopy(broker.fills), + "trades": copy.deepcopy(broker.trades), + "partial_orders": copy.deepcopy(broker._partial_orders), + "filled_this_bar": copy.deepcopy(broker._filled_this_bar), + } + + +@pytest.mark.parametrize( + "field", + [ + "commission_rate", + "commission_per_share", + "commission_per_trade", + "commission_minimum", + "slippage_rate", + "slippage_fixed", + "slippage_spread", + "stop_slippage_rate", + ], +) +@pytest.mark.parametrize("value", [-0.01, math.nan, math.inf]) +def test_engine_rejects_invalid_builtin_cost_config(field: str, value: float) -> None: + config = BacktestConfig(**{field: value}) + + with pytest.raises(ValueError, match=field): + Engine(DataFeed(prices_df=_prices()), _NoOpStrategy(), config) + + +@pytest.mark.parametrize("value", [-0.01, math.nan, math.inf]) +def test_engine_rejects_invalid_asset_spread(value: float) -> None: + config = BacktestConfig(slippage_spread_by_asset={"AAPL": value}) + + with pytest.raises(ValueError, match=r"slippage_spread_by_asset\['AAPL'\]"): + Engine(DataFeed(prices_df=_prices()), _NoOpStrategy(), config) + + +def test_run_backtest_enforces_config_validation() -> None: + config = BacktestConfig(commission_per_trade=-10.0) + + with pytest.raises(ValueError, match=r"commission_per_trade.*-10\.0"): + run_backtest(_prices(), _NoOpStrategy(), config=config) + + +@pytest.mark.parametrize("value", [-1.0, math.nan, math.inf]) +def test_invalid_custom_commission_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + engine = Engine(DataFeed(prices_df=_prices()), strategy) + model = _ConstantCommission(value) + engine.broker.commission_model = model + engine.broker.gatekeeper.commission_model = model + + with pytest.raises(ValueError, match=r"commission.*_ConstantCommission"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +@pytest.mark.parametrize("value", [-1.0, math.nan, math.inf]) +def test_invalid_custom_slippage_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + engine = Engine(DataFeed(prices_df=_prices()), strategy) + engine.broker.slippage_model = _ConstantSlippage(value) + + with pytest.raises(ValueError, match=r"slippage.*_ConstantSlippage"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +@pytest.mark.parametrize("value", [-1.0, math.nan, math.inf]) +def test_invalid_custom_impact_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + engine = Engine( + DataFeed(prices_df=_prices()), + strategy, + market_impact_model=_ConstantImpact(value), + ) + + with pytest.raises(ValueError, match=r"market impact.*_ConstantImpact"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +def test_invalid_execution_price_is_fail_atomic() -> None: + strategy = _RoundTripStrategy() + impact = _InvalidSellPriceImpact() + engine = Engine( + DataFeed(prices_df=_prices()), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + market_impact_model=impact, + ) + impact.broker = engine.broker + + with pytest.raises(ValueError, match=r"execution price.*-100\.0"): + engine.run() + + assert impact.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == impact.broker_snapshot diff --git a/tests/test_datafeed_memory.py b/tests/test_datafeed_memory.py index e6977517..423e2e0b 100644 --- a/tests/test_datafeed_memory.py +++ b/tests/test_datafeed_memory.py @@ -9,10 +9,10 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import BacktestConfig, DataFeed from ml4t.backtest.config import DataFrequency -from ml4t.specs.market_data import FeedSpec class TestDataFeedMemoryEfficiency: diff --git a/tests/test_equity_curve.py b/tests/test_equity_curve.py index 65223c8b..72bae584 100644 --- a/tests/test_equity_curve.py +++ b/tests/test_equity_curve.py @@ -3,11 +3,11 @@ from datetime import datetime, timedelta import polars as pl +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import BacktestConfig, DataFeed, Engine, Strategy from ml4t.backtest.analytics.equity import EquityCurve from ml4t.backtest.config import DataFrequency -from ml4t.specs.market_data import FeedSpec class TestEquityCurveAnnualization: diff --git a/tests/test_result.py b/tests/test_result.py index ece09e32..d1347538 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -10,6 +10,7 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.config import BacktestConfig from ml4t.backtest.result import ( @@ -17,7 +18,6 @@ enrich_trades_with_signals, ) from ml4t.backtest.types import Fill, OrderSide, Trade -from ml4t.specs.market_data import FeedSpec @pytest.fixture diff --git a/tests/test_strategy_templates.py b/tests/test_strategy_templates.py index fdf7b27a..966cc7b2 100644 --- a/tests/test_strategy_templates.py +++ b/tests/test_strategy_templates.py @@ -4,6 +4,7 @@ import numpy as np import polars as pl +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import BacktestConfig, DataFeed, Engine from ml4t.backtest.execution.schedule import RebalanceSchedule @@ -13,7 +14,6 @@ MomentumStrategy, SignalFollowingStrategy, ) -from ml4t.specs.market_data import FeedSpec def make_price_data( From 372f3c6b7e570f2bf9068eb5b07668d339381f1e Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:13:37 -0400 Subject: [PATCH 02/15] fix(accounting): reconcile partial-close costs Record partial exits as realized trades, allocate entry commission between exited and residual quantities, and enforce commission conservation with precision-derived tolerances. Closes #42. --- src/ml4t/backtest/execution/fill_executor.py | 45 +++++++- tests/contracts/test_book_parity_behaviors.py | 9 +- tests/helpers/invariants.py | 52 +++++---- tests/test_partial_close_accounting.py | 101 ++++++++++++++++++ 4 files changed, 178 insertions(+), 29 deletions(-) create mode 100644 tests/test_partial_close_accounting.py diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index 6b36d42e..c65aa6cd 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -252,6 +252,7 @@ def execute(self, order: Order, base_price: float) -> bool: # Update position and get actual commission (may change for flips) actual_commission = self._update_position(ctx) + fill.commission = actual_commission # Update cash (include multiplier for futures/derivatives) multiplier = broker.get_multiplier(order.asset) @@ -598,14 +599,52 @@ def _scale_position( else: # Short position pnl = (pos.entry_price - ctx.fill_price) * exited_qty * pos.multiplier - # Subtract proportional commission - # entry_commission is for the full position, so we take the proportional part - exit_portion_ratio = exited_qty / abs(pos.initial_quantity or old_qty) + # Allocate the current position's entry costs in proportion to the quantity + # removed. The residual cost remains attached to the residual position. + exit_portion_ratio = exited_qty / abs(old_qty) proportional_entry_commission = pos.entry_commission * exit_portion_ratio + pos.entry_commission -= proportional_entry_commission partial_exit_commission = ctx.commission total_commission = proportional_entry_commission + partial_exit_commission pnl -= total_commission + raw_pct = ( + (ctx.fill_price - pos.entry_price) / pos.entry_price if pos.entry_price else 0.0 + ) + pnl_pct = raw_pct if old_qty > 0 else -raw_pct + entry_quote = pos.context.get("entry_quote_context", {}) + exit_quote = ctx.quote_context + broker.trades.append( + Trade( + symbol=ctx.order.asset, + entry_time=pos.entry_time, + exit_time=ctx.current_time, + entry_price=pos.entry_price, + exit_price=ctx.fill_price, + quantity=math.copysign(exited_qty, old_qty), + pnl=pnl, + pnl_percent=pnl_pct, + bars_held=pos.bars_held, + fees=total_commission, + exit_slippage=ctx.slippage, + exit_reason=_get_exit_reason(ctx.order), + mfe=pos.max_favorable_excursion, + mae=pos.max_adverse_excursion, + entry_slippage=pos.entry_slippage, + multiplier=pos.multiplier, + entry_quote_mid_price=entry_quote.get("quote_mid_price"), + entry_bid_price=entry_quote.get("bid_price"), + entry_ask_price=entry_quote.get("ask_price"), + entry_spread=entry_quote.get("spread"), + entry_available_size=entry_quote.get("available_size"), + exit_quote_mid_price=exit_quote.get("quote_mid_price"), + exit_bid_price=exit_quote.get("bid_price"), + exit_ask_price=exit_quote.get("ask_price"), + exit_spread=exit_quote.get("spread"), + exit_available_size=exit_quote.get("available_size"), + ) + ) + # Record P&L event for trading stats broker._record_pnl_event(ctx.order.asset, pnl) diff --git a/tests/contracts/test_book_parity_behaviors.py b/tests/contracts/test_book_parity_behaviors.py index d7f19ecb..e5728a32 100644 --- a/tests/contracts/test_book_parity_behaviors.py +++ b/tests/contracts/test_book_parity_behaviors.py @@ -63,7 +63,6 @@ def on_data(self, timestamp, data, context, broker) -> None: self.msft_order_qty = order.quantity -@pytest.mark.no_invariant_check # Known: partial close during rebalance doesn't prorate entry commission def test_snapshot_value_freezes_targets_vs_incremental_recompute() -> None: start = datetime(2024, 1, 1) prices = pl.DataFrame( @@ -86,10 +85,14 @@ def test_snapshot_value_freezes_targets_vs_incremental_recompute() -> None: snapshot_strategy = _RebalanceByMode(RebalanceMode.SNAPSHOT) incremental_strategy = _RebalanceByMode(RebalanceMode.INCREMENTAL) - run_backtest(prices=prices, strategy=snapshot_strategy, config=cfg) - run_backtest(prices=prices, strategy=incremental_strategy, config=cfg) + snapshot_result = run_backtest(prices=prices, strategy=snapshot_strategy, config=cfg) + incremental_result = run_backtest(prices=prices, strategy=incremental_strategy, config=cfg) assert snapshot_strategy.msft_order_qty > incremental_strategy.msft_order_qty + for result in (snapshot_result, incremental_result): + fill_costs = sum(fill.commission for fill in result.fills) + reported_costs = sum(trade.fees for trade in result.trades) + assert reported_costs == pytest.approx(fill_costs, abs=1e-9) class _RotateSellThenBuy(Strategy): diff --git a/tests/helpers/invariants.py b/tests/helpers/invariants.py index e00d3514..bf0c735d 100644 --- a/tests/helpers/invariants.py +++ b/tests/helpers/invariants.py @@ -34,6 +34,7 @@ def assert_result_invariants( check_no_nan: bool = True, check_exit_reason_consistency: bool = True, check_fill_order_type_bounds: bool = True, + check_commission_allocation: bool = True, ) -> None: """Assert universal invariants on a BacktestResult. @@ -62,6 +63,14 @@ def assert_result_invariants( _check_exit_reason_consistency(result.trades) if check_fill_order_type_bounds: _check_fill_order_type_bounds(result) + if check_commission_allocation: + _check_commission_allocation(result) + + +def _accounting_tolerance(*values: float, operations: int = 1) -> float: + """Bound rounding error by the represented values and arithmetic operation count.""" + scale = max((abs(value) for value in values), default=0.0) + return max(1e-9, math.ulp(scale) * max(16, operations * 4)) def _check_equity_terminal( @@ -69,14 +78,7 @@ def _check_equity_terminal( initial_cash: float, closed_trades: list, ) -> None: - """Verify: initial_cash + sum(closed_pnl) + sum(open_pnl) ≈ final_value. - - When open positions exist, the tolerance is expanded because the open trade - PnL is computed from Position state which may not perfectly capture all - intermediate costs (especially in rebalancing with integer shares and high - commission rates). Multi-asset rebalancing with integer shares also creates - small rounding discrepancies in position PnL vs. cash-based equity tracking. - """ + """Verify: initial_cash + sum(closed_pnl) + sum(open_pnl) ≈ final_value.""" if not result.equity_curve: return @@ -88,20 +90,8 @@ def _check_equity_terminal( expected = initial_cash + closed_pnl + open_pnl diff = abs(expected - final_value) - # Base tolerance: relative to portfolio size - tol = max(_ABS_TOL, abs(final_value) * 1e-6) - - # Expand tolerance for total fill costs (commission + slippage on all fills) - total_fill_costs = sum(f.commission + f.slippage for f in result.fills) - if total_fill_costs > 0: - tol = max(tol, total_fill_costs * 0.05) # 5% of total costs - - # Expand tolerance for open positions: mark-to-market PnL from Position state - # can diverge slightly from cash-based equity tracking, especially with - # multi-asset rebalancing and integer share rounding. - if open_trades: - open_notional = sum(abs(t.quantity) * t.exit_price * t.multiplier for t in open_trades) - tol = max(tol, open_notional * 1e-4) # 0.01% of open notional + terms = [initial_cash, *(t.pnl for t in closed_trades), *(t.pnl for t in open_trades)] + tol = _accounting_tolerance(expected, final_value, *terms, operations=len(terms) + 1) assert diff <= tol, ( f"Equity terminal invariant violated: " @@ -118,7 +108,7 @@ def _check_pnl_decomposition(closed_trades: list) -> None: expected_net = gross - t.fees diff = abs(expected_net - t.pnl) - tol = max(_ABS_TOL, abs(gross) * 1e-6) + tol = _accounting_tolerance(gross, t.fees, expected_net, t.pnl, operations=2) assert diff <= tol, ( f"PnL decomposition invariant violated for trade {i} ({t.symbol}): " f"gross_pnl({gross:.6f}) - fees({t.fees:.6f}) = {expected_net:.6f} " @@ -295,3 +285,19 @@ def _check_fill_order_type_bounds(result: BacktestResult) -> None: f"Fill order-type bound violated for fill {i} ({f.asset}): " f"stop SELL filled at {f.price:.6f} > stop_price {stop_price:.6f}" ) + + +def _check_commission_allocation(result: BacktestResult) -> None: + """Verify every charged fill commission is allocated to a realized or open trade.""" + fill_commission = sum(fill.commission for fill in result.fills) + trade_commission = sum(trade.fees for trade in result.trades) + tol = _accounting_tolerance( + fill_commission, + trade_commission, + operations=len(result.fills) + len(result.trades), + ) + assert abs(fill_commission - trade_commission) <= tol, ( + "Commission allocation invariant violated: " + f"fills({fill_commission:.12f}) != trades({trade_commission:.12f}), " + f"diff={abs(fill_commission - trade_commission):.12f}, tol={tol:.12f}" + ) diff --git a/tests/test_partial_close_accounting.py b/tests/test_partial_close_accounting.py new file mode 100644 index 00000000..1afa0d70 --- /dev/null +++ b/tests/test_partial_close_accounting.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +from datetime import datetime, timedelta + +import pytest + +from ml4t.backtest import AssetClass, Broker, ContractSpec, OrderSide, OrderStatus +from ml4t.backtest.config import ShareType +from ml4t.backtest.models import NoSlippage, PercentageCommission + + +def _execute(broker: Broker, *, day: int, price: float, quantity: float) -> None: + timestamp = datetime(2024, 1, 1) + timedelta(days=day) + broker._update_time( + timestamp=timestamp, + prices={"TEST": price}, + opens={"TEST": price}, + highs={"TEST": price}, + lows={"TEST": price}, + volumes={"TEST": 1_000_000.0}, + signals={}, + ) + side = OrderSide.BUY if quantity > 0 else OrderSide.SELL + order = broker.submit_order("TEST", abs(quantity), side) + assert order is not None + broker._process_orders() + assert order.status == OrderStatus.FILLED + + +@pytest.mark.parametrize( + ("share_type", "contract_specs", "steps", "expected_closed", "expected_remaining"), + [ + ( + ShareType.INTEGER, + None, + [(100.0, 100.0), (110.0, -30.0), (90.0, -20.0)], + 2, + 50.0, + ), + ( + ShareType.FRACTIONAL, + None, + [(100.0, -10.5), (90.0, 3.25), (110.0, 2.0)], + 2, + -5.25, + ), + ( + ShareType.FRACTIONAL, + None, + [(100.0, 10.0), (110.0, 5.0), (120.0, -6.0)], + 1, + 9.0, + ), + ( + ShareType.INTEGER, + None, + [(100.0, 10.0), (120.0, -15.0)], + 1, + -5.0, + ), + ( + ShareType.INTEGER, + {"TEST": ContractSpec("TEST", AssetClass.FUTURE, multiplier=50.0)}, + [(4_000.0, 4.0), (4_010.0, -1.0)], + 1, + 3.0, + ), + ], +) +def test_fill_costs_are_conserved_across_realized_and_residual_positions( + share_type: ShareType, + contract_specs: dict[str, ContractSpec] | None, + steps: list[tuple[float, float]], + expected_closed: int, + expected_remaining: float, +) -> None: + broker = Broker( + initial_cash=10_000_000.0, + commission_model=PercentageCommission(0.01), + slippage_model=NoSlippage(), + allow_short_selling=True, + allow_leverage=True, + share_type=share_type, + contract_specs=contract_specs, + ) + + for day, (price, quantity) in enumerate(steps): + _execute(broker, day=day, price=price, quantity=quantity) + + position = broker.get_position("TEST") + assert position is not None + assert position.quantity == pytest.approx(expected_remaining) + assert len(broker.trades) == expected_closed + + fill_costs = sum(fill.commission for fill in broker.fills) + realized_costs = sum(trade.fees for trade in broker.trades) + residual_costs = position.entry_commission + assert realized_costs + residual_costs == pytest.approx(fill_costs, abs=1e-9) + + for trade in broker.trades: + assert trade.gross_pnl - trade.fees == pytest.approx(trade.pnl, abs=1e-9) From 1c70dde3efb508b9369875b36532d8fc932c351d Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:22:52 -0400 Subject: [PATCH 03/15] feat(results): preserve rejected orders Expose structured rejected-order records, aggregate counts, DataFrame conversion, and additive Parquet round trips while retaining original requested quantities. Closes #43. --- src/ml4t/backtest/engine.py | 12 ++- src/ml4t/backtest/export.py | 4 + src/ml4t/backtest/result.py | 95 ++++++++++++++++++++- src/ml4t/backtest/types.py | 25 ++++++ tests/benchmark/test_hotpath_benchmarks.py | 1 + tests/test_rejected_order_results.py | 99 ++++++++++++++++++++++ 6 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 tests/test_rejected_order_results.py diff --git a/src/ml4t/backtest/engine.py b/src/ml4t/backtest/engine.py index 47b57b06..db7a13e1 100644 --- a/src/ml4t/backtest/engine.py +++ b/src/ml4t/backtest/engine.py @@ -298,6 +298,8 @@ def _build_activity_metrics(self) -> dict[str, int | float]: ) max_open_positions = max((state[5] for state in self.portfolio_state), default=0) return { + "num_orders": len(self.broker.orders), + "num_rejected_orders": len(self.broker.get_rejected_orders()), "num_fills": 0, "num_rebalance_events": 0, "unique_symbols_traded": 0, @@ -335,6 +337,8 @@ def _build_activity_metrics(self) -> dict[str, int | float]: max_open_positions = max((state[5] for state in self.portfolio_state), default=0) return { + "num_orders": len(self.broker.orders), + "num_rejected_orders": len(self.broker.get_rejected_orders()), "num_fills": len(fills), "num_rebalance_events": len(rebalance_events), "unique_symbols_traded": len(traded_symbols), @@ -356,9 +360,14 @@ def _generate_results(self) -> BacktestResult: trades=[], equity_curve=[], fills=[], + rejected_orders=self.broker.get_rejected_orders(), predictions=self.feed.signals, portfolio_state=[], - metrics={"skipped_bars": self._skipped_bars}, + metrics={ + "skipped_bars": self._skipped_bars, + "num_orders": len(self.broker.orders), + "num_rejected_orders": len(self.broker.get_rejected_orders()), + }, config=self.config, ) @@ -475,6 +484,7 @@ def _generate_results(self) -> BacktestResult: trades=all_trades, # Includes both closed and open trades equity_curve=self.equity_curve, fills=self.broker.fills, + rejected_orders=self.broker.get_rejected_orders(), predictions=self.feed.signals, portfolio_state=self.portfolio_state, metrics=metrics, diff --git a/src/ml4t/backtest/export.py b/src/ml4t/backtest/export.py index 514d0393..19fc1894 100644 --- a/src/ml4t/backtest/export.py +++ b/src/ml4t/backtest/export.py @@ -130,6 +130,8 @@ def batch_export( record["total_commission"] = metrics.get("total_commission", 0.0) record["total_slippage"] = metrics.get("total_slippage", 0.0) record["num_fills"] = metrics.get("num_fills", 0) + record["num_orders"] = metrics.get("num_orders", 0) + record["num_rejected_orders"] = metrics.get("num_rejected_orders", 0) record["num_rebalance_events"] = metrics.get("num_rebalance_events", 0) record["unique_symbols_traded"] = metrics.get("unique_symbols_traded", 0) record["total_filled_notional"] = metrics.get("total_filled_notional", 0.0) @@ -217,6 +219,8 @@ def generate_json_report( "win_rate": metrics.get("win_rate", 0.0), "profit_factor": metrics.get("profit_factor", 0.0), "final_value": metrics.get("final_value", 0.0), + "num_orders": metrics.get("num_orders", 0), + "num_rejected_orders": metrics.get("num_rejected_orders", 0), } ) diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 34f70419..37c53ab5 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -35,7 +35,7 @@ except ImportError: # pragma: no cover - fallback for local editable edge cases __version__ = "0.0.0.dev0" from .analytics.annualization import should_session_align -from .types import Fill, OrderSide, Trade +from .types import Fill, Order, OrderSide, OrderStatus, OrderType, Trade if TYPE_CHECKING: from .analytics import EquityCurve, TradeAnalyzer @@ -56,6 +56,7 @@ class BacktestResult: trades: List of completed Trade objects equity_curve: List of (timestamp, portfolio_value) tuples fills: List of Fill objects (all order fills) + rejected_orders: Orders that reached the rejected terminal state predictions: Raw prediction DataFrame passed into the backtest (optional) metrics: Dictionary of computed performance metrics config: BacktestConfig used for the backtest (optional) @@ -74,12 +75,14 @@ class BacktestResult: portfolio_state: list[tuple[datetime, float, float, float, float, int]] = field( default_factory=list ) + rejected_orders: list[Order] = field(default_factory=list) # Cached DataFrames (computed on demand) _trades_df: pl.DataFrame | None = field(default=None, repr=False) _equity_df: pl.DataFrame | None = field(default=None, repr=False) _fills_df: pl.DataFrame | None = field(default=None, repr=False) _portfolio_state_df: pl.DataFrame | None = field(default=None, repr=False) + _rejected_orders_df: pl.DataFrame | None = field(default=None, repr=False) def _feed_spec(self) -> FeedSpec | None: if self.config is None: @@ -204,6 +207,38 @@ def to_fills_dataframe(self) -> pl.DataFrame: self._fills_df = pl.DataFrame(records, schema=self._fills_schema()) return self._fills_df + def to_rejected_orders_dataframe(self) -> pl.DataFrame: + """Convert rejected orders to a stable, machine-readable DataFrame.""" + if self._rejected_orders_df is not None: + return self._rejected_orders_df + if not self.rejected_orders: + return pl.DataFrame(schema=self._rejected_orders_schema()) + + records = [ + { + "order_id": order.order_id, + "symbol": order.asset, + "timestamp": order.created_at, + "requested_quantity": order.requested_quantity, + "side": order.side.value, + "order_type": order.order_type.value, + "limit_price": order.limit_price, + "stop_price": order.stop_price, + "trail_amount": order.trail_amount, + "parent_id": order.parent_id, + "rebalance_id": order.rebalance_id, + "status": order.status.value, + "rejection_code": order.rejection_code, + "rejection_reason": order.rejection_reason, + } + for order in self.rejected_orders + ] + self._rejected_orders_df = pl.DataFrame( + records, + schema=self._rejected_orders_schema(), + ) + return self._rejected_orders_df + def to_predictions_dataframe(self) -> pl.DataFrame: """Return the raw prediction DataFrame used as backtest input.""" if self.predictions is None: @@ -524,8 +559,8 @@ def to_parquet( Args: path: Directory path to write files include: Components to include. Default: all. - Options: ["trades", "fills", "predictions", "equity", "portfolio_state", - "daily_pnl", "metrics", "config", "spec"] + Options: ["trades", "fills", "rejected_orders", "predictions", "equity", + "portfolio_state", "daily_pnl", "metrics", "config", "spec"] compression: Parquet compression codec (default: "zstd") Returns: @@ -538,6 +573,7 @@ def to_parquet( include = [ "trades", "fills", + "rejected_orders", "predictions", "equity", "portfolio_state", @@ -559,6 +595,14 @@ def to_parquet( self.to_fills_dataframe().write_parquet(fills_path, compression=compression) written["fills"] = fills_path + if "rejected_orders" in include: + rejected_orders_path = path / "rejected_orders.parquet" + self.to_rejected_orders_dataframe().write_parquet( + rejected_orders_path, + compression=compression, + ) + written["rejected_orders"] = rejected_orders_path + if "predictions" in include and self.predictions is not None: predictions_path = path / "predictions.parquet" self.to_predictions_dataframe().write_parquet(predictions_path, compression=compression) @@ -725,6 +769,30 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: ) ) + rejected_orders: list[Order] = [] + rejected_orders_path = path / "rejected_orders.parquet" + if rejected_orders_path.exists(): + rejected_orders_df = pl.read_parquet(rejected_orders_path) + for row in rejected_orders_df.iter_rows(named=True): + rejected_orders.append( + Order( + order_id=row["order_id"], + asset=row["symbol"], + created_at=row["timestamp"], + requested_quantity=row["requested_quantity"], + quantity=row["requested_quantity"], + side=OrderSide(row["side"]), + order_type=OrderType(row["order_type"]), + limit_price=row.get("limit_price"), + stop_price=row.get("stop_price"), + trail_amount=row.get("trail_amount"), + parent_id=row.get("parent_id"), + rebalance_id=row.get("rebalance_id"), + status=OrderStatus(row["status"]), + rejection_reason=row.get("rejection_reason"), + ) + ) + predictions = None predictions_path = path / "predictions.parquet" if predictions_path.exists(): @@ -785,6 +853,7 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: fills=fills, predictions=predictions, portfolio_state=portfolio_state, + rejected_orders=rejected_orders, metrics=metrics, config=config, ) @@ -862,6 +931,26 @@ def _fills_schema() -> dict[str, pl.DataType]: "available_size": pl.Float64(), } + @staticmethod + def _rejected_orders_schema() -> dict[str, pl.DataType]: + """Schema for rejected order records added compatibly in v0.1.0.""" + return { + "order_id": pl.String(), + "symbol": pl.String(), + "timestamp": pl.Datetime(), + "requested_quantity": pl.Float64(), + "side": pl.String(), + "order_type": pl.String(), + "limit_price": pl.Float64(), + "stop_price": pl.Float64(), + "trail_amount": pl.Float64(), + "parent_id": pl.String(), + "rebalance_id": pl.String(), + "status": pl.String(), + "rejection_code": pl.String(), + "rejection_reason": pl.String(), + } + @staticmethod def _equity_schema() -> dict[str, pl.DataType]: """Schema for equity DataFrame.""" diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index b4e9ae76..24264fd5 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -164,6 +164,7 @@ class Order: filled_price: float | None = None filled_quantity: float = 0.0 rejection_reason: str | None = None # Reason if order was rejected + requested_quantity: float | None = None # Internal risk management fields (set by broker) _created_bar_index: int = 0 _signal_price: float | None = None # Close price at order creation time @@ -171,6 +172,30 @@ class Order: _exit_reason: ExitReason | None = None # Typed exit reason (preferred) _risk_fill_price: float | None = None # Stop/target price for risk exits + def __post_init__(self) -> None: + if self.requested_quantity is None: + self.requested_quantity = self.quantity + + @property + def rejection_code(self) -> str | None: + """Return a stable machine-readable category for the rejection reason.""" + if self.status is not OrderStatus.REJECTED: + return None + reason = (self.rejection_reason or "").lower() + if "rounds to zero" in reason: + return "quantity_rounds_to_zero" + if "no price" in reason: + return "price_unavailable" + if "fill check" in reason: + return "fill_check_failed" + if "short" in reason or "reversal not allowed" in reason: + return "account_restriction" + if "buying power" in reason or "margin" in reason: + return "insufficient_buying_power" + if "cash" in reason or "insufficient" in reason: + return "insufficient_cash" + return "order_validation_failed" + @dataclass class Position: diff --git a/tests/benchmark/test_hotpath_benchmarks.py b/tests/benchmark/test_hotpath_benchmarks.py index 0e7b0fe4..4f291176 100644 --- a/tests/benchmark/test_hotpath_benchmarks.py +++ b/tests/benchmark/test_hotpath_benchmarks.py @@ -175,6 +175,7 @@ def test_optimized_feed_matches_legacy_output(): @pytest.mark.benchmark +@pytest.mark.no_cover def test_optimized_feed_runtime_vs_legacy_baseline(): prices, signals = _build_benchmark_data(n_bars=3000, n_assets=20) diff --git a/tests/test_rejected_order_results.py b/tests/test_rejected_order_results.py new file mode 100644 index 00000000..d763f3eb --- /dev/null +++ b/tests/test_rejected_order_results.py @@ -0,0 +1,99 @@ +from __future__ import annotations + +from datetime import datetime + +import polars as pl + +from ml4t.backtest import BacktestConfig, Strategy, run_backtest +from ml4t.backtest.types import ExecutionMode + + +class _UnaffordableOrder(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + broker.submit_order("AAPL", 1_000_000.0) + + +class _NoOrders(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + pass + + +def _prices() -> pl.DataFrame: + return pl.DataFrame( + { + "timestamp": [datetime(2024, 1, 2)], + "asset": ["AAPL"], + "open": [100.0], + "high": [100.0], + "low": [100.0], + "close": [100.0], + "volume": [1_000_000.0], + } + ) + + +def _run(strategy: Strategy): + return run_backtest( + prices=_prices(), + strategy=strategy, + config=BacktestConfig( + initial_cash=10_000.0, + execution_mode=ExecutionMode.SAME_BAR, + ), + ) + + +def test_unaffordable_order_is_preserved_in_public_result() -> None: + result = _run(_UnaffordableOrder()) + + assert len(result.rejected_orders) == 1 + rejected = result.rejected_orders[0] + assert rejected.order_id + assert rejected.asset == "AAPL" + assert rejected.created_at == datetime(2024, 1, 2) + assert rejected.requested_quantity == 1_000_000.0 + assert rejected.status.value == "rejected" + assert rejected.rejection_code == "insufficient_cash" + assert rejected.rejection_reason + assert result.metrics["num_orders"] == 1 + assert result.metrics["num_rejected_orders"] == 1 + assert result.fills == [] + assert result.equity_curve[-1][1] == 10_000.0 + + +def test_no_orders_and_all_orders_rejected_are_distinguishable() -> None: + no_orders = _run(_NoOrders()) + all_rejected = _run(_UnaffordableOrder()) + + assert no_orders.metrics["num_orders"] == 0 + assert no_orders.metrics["num_rejected_orders"] == 0 + assert all_rejected.metrics["num_orders"] == 1 + assert all_rejected.metrics["num_rejected_orders"] == 1 + + +def test_rejected_orders_round_trip_through_result_artifact(tmp_path) -> None: + result = _run(_UnaffordableOrder()) + + frame = result.to_rejected_orders_dataframe() + assert frame.to_dicts() == [ + { + "order_id": result.rejected_orders[0].order_id, + "symbol": "AAPL", + "timestamp": datetime(2024, 1, 2), + "requested_quantity": 1_000_000.0, + "side": "buy", + "order_type": "market", + "limit_price": None, + "stop_price": None, + "trail_amount": None, + "parent_id": None, + "rebalance_id": None, + "status": "rejected", + "rejection_code": "insufficient_cash", + "rejection_reason": result.rejected_orders[0].rejection_reason, + } + ] + + result.to_parquet(tmp_path) + loaded = type(result).from_parquet(tmp_path) + assert loaded.to_rejected_orders_dataframe().to_dicts() == frame.to_dicts() From a2940c2e0db66a497665a7ecb92f8442563d30fc Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:25:31 -0400 Subject: [PATCH 04/15] fix(risk): preserve liquidation causes Attach liquidation metadata before order submission and carry the category and detailed cause through fills, trades, DataFrames, and Parquet artifacts. Closes #44. --- src/ml4t/backtest/broker.py | 12 +++-- src/ml4t/backtest/execution/fill_executor.py | 5 ++ src/ml4t/backtest/result.py | 10 ++++ src/ml4t/backtest/types.py | 3 ++ tests/risk/test_portfolio_manager.py | 49 ++++++++++++++++++++ tests/test_result.py | 3 ++ 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 1fdcccb6..1e2fc551 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -1052,11 +1052,17 @@ def flatten_all_positions( liquidations: list[Order] = [] for asset in list(self.positions): - order = self.close_position(asset, order_type=order_type) + order = self.close_position( + asset, + order_type=order_type, + _options=SubmitOrderOptions( + eligible_in_next_bar_mode=True, + risk_exit_reason=reason, + exit_reason=ExitReason.RISK_LIQUIDATION, + ), + ) if order is None: continue - order._exit_reason = ExitReason.RISK_LIQUIDATION - order._risk_exit_reason = reason liquidations.append(order) return liquidations diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index c65aa6cd..3c1ef77e 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -221,6 +221,8 @@ def execute(self, order: Order, base_price: float) -> bool: bid_size=quote_context["bid_size"], ask_size=quote_context["ask_size"], available_size=quote_context["available_size"], + exit_reason=_get_exit_reason(order), + exit_reason_detail=order._risk_exit_reason, ) broker.fills.append(fill) @@ -461,6 +463,7 @@ def _close_position(self, ctx: FillContext, pos: Position, old_qty: float) -> No fees=total_commission, exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(order), + exit_reason_detail=order._risk_exit_reason, mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, @@ -527,6 +530,7 @@ def _flip_position( fees=total_close_commission, exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(order), + exit_reason_detail=order._risk_exit_reason, mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, @@ -628,6 +632,7 @@ def _scale_position( fees=total_commission, exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(ctx.order), + exit_reason_detail=ctx.order._risk_exit_reason, mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 37c53ab5..f2a0a00d 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -161,6 +161,7 @@ def to_trades_dataframe(self) -> pl.DataFrame: "total_slippage_cost": t.total_slippage_cost, "cost_drag": t.cost_drag, "exit_reason": t.exit_reason, + "exit_reason_detail": t.exit_reason_detail, "status": t.status, } ) @@ -201,6 +202,8 @@ def to_fills_dataframe(self) -> pl.DataFrame: "bid_size": fill.bid_size, "ask_size": fill.ask_size, "available_size": fill.available_size, + "exit_reason": fill.exit_reason, + "exit_reason_detail": fill.exit_reason_detail, } ) @@ -706,6 +709,8 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: fees=fees, exit_slippage=row.get("exit_slippage", row.get("slippage", 0.0)), exit_reason=row.get("exit_reason", "signal"), + exit_reason_detail=row.get("exit_reason_detail"), + status=row.get("status", "closed"), mfe=row["mfe"], mae=row["mae"], entry_slippage=row.get("entry_slippage", 0.0), @@ -766,6 +771,8 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: bid_size=row.get("bid_size"), ask_size=row.get("ask_size"), available_size=row.get("available_size"), + exit_reason=row.get("exit_reason", ""), + exit_reason_detail=row.get("exit_reason_detail"), ) ) @@ -901,6 +908,7 @@ def _trades_schema() -> dict[str, pl.DataType]: "total_slippage_cost": pl.Float64(), "cost_drag": pl.Float64(), "exit_reason": pl.String(), + "exit_reason_detail": pl.String(), "status": pl.String(), # "closed" or "open" } @@ -929,6 +937,8 @@ def _fills_schema() -> dict[str, pl.DataType]: "bid_size": pl.Float64(), "ask_size": pl.Float64(), "available_size": pl.Float64(), + "exit_reason": pl.String(), + "exit_reason_detail": pl.String(), } @staticmethod diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index 24264fd5..fd7657f5 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -396,6 +396,8 @@ class Fill: bid_size: float | None = None ask_size: float | None = None available_size: float | None = None + exit_reason: str = "" + exit_reason_detail: str | None = None @dataclass @@ -428,6 +430,7 @@ class Trade: exit_slippage: float = 0.0 # Per-unit slippage on exit # Exit reason for trade analysis (cross-library API field) exit_reason: str = "signal" # ExitReason enum value as string + exit_reason_detail: str | None = None # Trade status: "closed" (actually exited) or "open" (mark-to-market at end) status: str = "closed" # MFE/MAE preserved from Position for trade analysis (shorter field names) diff --git a/tests/risk/test_portfolio_manager.py b/tests/risk/test_portfolio_manager.py index 80ef6d8a..1e214303 100644 --- a/tests/risk/test_portfolio_manager.py +++ b/tests/risk/test_portfolio_manager.py @@ -4,6 +4,7 @@ import pytest +from ml4t.backtest import BacktestResult from ml4t.backtest.broker import Broker from ml4t.backtest.models import NoCommission, NoSlippage from ml4t.backtest.risk.portfolio.limits import ( @@ -195,6 +196,54 @@ def test_update_liquidate_action_is_idempotent_with_broker(self): assert len(pending) == 1 assert pending[0]._exit_reason == ExitReason.RISK_LIQUIDATION + @pytest.mark.parametrize("immediate_fill", [True, False]) + def test_liquidation_cause_survives_fill_trade_and_artifact( + self, + immediate_fill: bool, + tmp_path, + ): + manager = RiskManager(limits=[MaxDrawdownLimit(max_drawdown=0.10)]) + manager.initialize(initial_equity=100000.0) + broker = Broker( + initial_cash=100000.0, + commission_model=NoCommission(), + slippage_model=NoSlippage(), + immediate_fill=immediate_fill, + ) + open_long_position(broker, "AAPL", 100.0, 150.0) + + manager.update(equity=85000.0, positions={"AAPL": 15000.0}, broker=broker) + manager.update(equity=84000.0, positions={"AAPL": 15000.0}, broker=broker) + if not immediate_fill: + broker._process_orders() + + liquidation_orders = [ + order for order in broker.orders if order._exit_reason == ExitReason.RISK_LIQUIDATION + ] + assert len(liquidation_orders) == 1 + detail = liquidation_orders[0]._risk_exit_reason + assert detail and "drawdown" in detail.lower() + + exit_fill = broker.fills[-1] + exit_trade = broker.trades[-1] + assert exit_fill.exit_reason == "risk_liquidation" + assert exit_fill.exit_reason_detail == detail + assert exit_trade.exit_reason == "risk_liquidation" + assert exit_trade.exit_reason_detail == detail + + result = BacktestResult( + trades=broker.trades, + equity_curve=[], + fills=broker.fills, + metrics={}, + ) + result.to_parquet(tmp_path) + loaded = BacktestResult.from_parquet(tmp_path) + assert loaded.fills[-1].exit_reason == "risk_liquidation" + assert loaded.fills[-1].exit_reason_detail == detail + assert loaded.trades[-1].exit_reason == "risk_liquidation" + assert loaded.trades[-1].exit_reason_detail == detail + def test_update_warn_action(self): """Test that warn action adds to warnings.""" limits = [MaxExposureLimit(max_exposure_pct=0.50, action="warn")] diff --git a/tests/test_result.py b/tests/test_result.py index d1347538..c8a88c83 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -202,6 +202,7 @@ def test_trades_dataframe_basic(self, backtest_result: BacktestResult): "total_slippage_cost", "cost_drag", "exit_reason", + "exit_reason_detail", "status", ] @@ -336,6 +337,8 @@ def test_fills_dataframe_basic(self, backtest_result: BacktestResult): "bid_size", "ask_size", "available_size", + "exit_reason", + "exit_reason_detail", ] assert df["rebalance_id"].to_list() == ["rebalance-1", "rebalance-1"] From 9d3df453f0260fb92f64cb71a75dbd7983601aaa Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:12:44 -0400 Subject: [PATCH 05/15] test: isolate runtime benchmark from coverage --- tests/benchmark/test_hotpath_benchmarks.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/benchmark/test_hotpath_benchmarks.py b/tests/benchmark/test_hotpath_benchmarks.py index 4f291176..08a234e5 100644 --- a/tests/benchmark/test_hotpath_benchmarks.py +++ b/tests/benchmark/test_hotpath_benchmarks.py @@ -158,6 +158,13 @@ def _legacy_view(assets: dict[str, dict[str, Any]]) -> dict[str, dict[str, Any]] } +def _coverage_is_active() -> bool: + """Return whether coverage.py is currently tracing this process.""" + from coverage import Coverage + + return Coverage.current() is not None + + @pytest.mark.benchmark def test_optimized_feed_matches_legacy_output(): prices, signals = _build_benchmark_data(n_bars=50, n_assets=5) @@ -175,8 +182,10 @@ def test_optimized_feed_matches_legacy_output(): @pytest.mark.benchmark -@pytest.mark.no_cover def test_optimized_feed_runtime_vs_legacy_baseline(): + if _coverage_is_active(): + pytest.skip("Runtime benchmark requires coverage instrumentation to be disabled") + prices, signals = _build_benchmark_data(n_bars=3000, n_assets=20) # Warm-up for consistent timing From 42e7b91dbc90f25991620eedb70f0db5659cfde0 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:20:41 -0400 Subject: [PATCH 06/15] fix: address milestone review findings --- docs/user-guide/results.md | 16 ++- src/ml4t/backtest/analytics/bridge.py | 2 + src/ml4t/backtest/broker.py | 1 - src/ml4t/backtest/execution/fill_executor.py | 32 +++-- src/ml4t/backtest/execution/impact.py | 5 +- src/ml4t/backtest/result.py | 19 ++- src/ml4t/backtest/types.py | 22 ++-- tests/contracts/test_book_parity_behaviors.py | 5 - tests/helpers/invariants.py | 34 ++++-- tests/risk/test_portfolio_manager.py | 1 + tests/test_broker.py | 36 ++++++ tests/test_cost_validation.py | 113 +++++++++++++++++- tests/test_partial_close_accounting.py | 59 ++++++++- tests/test_rejected_order_results.py | 81 ++++++++++++- tests/test_result.py | 3 + 15 files changed, 372 insertions(+), 57 deletions(-) diff --git a/docs/user-guide/results.md b/docs/user-guide/results.md index d7791b88..a8982f2c 100644 --- a/docs/user-guide/results.md +++ b/docs/user-guide/results.md @@ -14,6 +14,8 @@ For reproducibility, `BacktestResult` also exposes: - `result.to_spec_dict()` for a richer runtime snapshot including library version and realized window - `result.to_predictions_dataframe()` for the raw prediction/input surface passed into the backtest, when available +- `result.rejected_orders` and `result.to_rejected_orders_dataframe()` for orders that reached + the rejected terminal state - `result.to_parquet(...)`, which writes `config.yaml`, `spec.yaml`, and `predictions.parquet` when available @@ -86,6 +88,8 @@ print(f"Net PF: {m['profit_factor']:.2f}") | `sortino` | Sortino ratio | | `calmar` | Calmar ratio | | `num_trades` | Total completed trades | +| `num_orders` | Total submitted orders | +| `num_rejected_orders` | Orders that reached the rejected terminal state | | `num_fills` | Total execution events | | `num_rebalance_events` | Unique timestamps with at least one fill | | `unique_symbols_traded` | Number of symbols with at least one fill | @@ -117,7 +121,7 @@ print(f"Net PF: {m['profit_factor']:.2f}") `BacktestResult` now exposes three distinct raw reporting surfaces: -- `trades`: flat-to-flat lifecycle summaries +- `trades`: realized exit legs plus end-of-backtest open-position marks - `fills`: execution blotter rows - `portfolio_state`: end-of-bar portfolio snapshots @@ -249,9 +253,12 @@ Returns a Polars DataFrame with columns: | `total_slippage_cost` | Float | Entry + exit slippage in dollars | | `cost_drag` | Float | Total cost as fraction of notional | | `exit_reason` | String | Why the trade exited | -| `status` | String | "closed" or "open" | +| `exit_reason_detail` | String | Detailed risk or liquidation cause, when available | +| `status` | String | "closed", "partial", or "open" | -Open positions at the end of the backtest are included with `status="open"` and mark-to-market values. +Partial reductions use `status="partial"`; lifecycle metrics exclude them to avoid counting one +position's holding period and excursions more than once. Open positions at the end of the backtest +use `status="open"` and mark-to-market values. ## Equity DataFrame @@ -352,6 +359,8 @@ Fill objects carry order-type metadata for audit: | `fill.spread` | Bid-ask spread | | `fill.bid_size` / `fill.ask_size` | Quote sizes | | `fill.available_size` | Side-aware size used for the fill context | +| `fill.exit_reason` | Typed exit category; empty for entry fills | +| `fill.exit_reason_detail` | Detailed risk or liquidation cause, when available | For quote-aware backtests, `fills.parquet` is the first place to look when you want to verify whether a result difference came from: @@ -380,6 +389,7 @@ result.to_parquet("./results/my_backtest") # Creates: # trades.parquet # fills.parquet +# rejected_orders.parquet # predictions.parquet # if raw prediction inputs were supplied # equity.parquet # portfolio_state.parquet diff --git a/src/ml4t/backtest/analytics/bridge.py b/src/ml4t/backtest/analytics/bridge.py index 14b275a1..a1a22c45 100644 --- a/src/ml4t/backtest/analytics/bridge.py +++ b/src/ml4t/backtest/analytics/bridge.py @@ -23,6 +23,8 @@ def to_trade_record(trade: Trade) -> dict[str, Any]: With the aligned schema (v0.1.0a6+), field names now match between backtest Trade and diagnostic TradeRecord, simplifying this conversion. + ``exit_reason_detail`` remains on the backtest record until the diagnostic + TradeRecord schema accepts that optional field. Args: trade: A completed Trade from backtest diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 1e2fc551..9fc7193a 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -1056,7 +1056,6 @@ def flatten_all_positions( asset, order_type=order_type, _options=SubmitOrderOptions( - eligible_in_next_bar_mode=True, risk_exit_reason=reason, exit_reason=ExitReason.RISK_LIQUIDATION, ), diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index 3c1ef77e..52f6ceb7 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -42,6 +42,11 @@ def _get_exit_reason(order: Order) -> str: return ExitReason.SIGNAL.value +def _is_position_flip(old_quantity: float, new_quantity: float) -> bool: + """Return whether a position crosses through zero to the opposite side.""" + return old_quantity > 0 > new_quantity or old_quantity < 0 < new_quantity + + @dataclass class FillContext: """Context for a single fill execution. @@ -99,6 +104,10 @@ def execute(self, order: Order, base_price: float) -> bool: Returns: True if order is fully filled, False if partially filled + + Raises: + ValueError: If an execution model returns a non-finite, non-positive, + negative, or directionally favorable value outside its contract. """ broker = self.broker current_time = broker._current_time @@ -125,14 +134,13 @@ def execute(self, order: Order, base_price: float) -> bool: ) fill_quantity = exec_result.fillable_quantity - if broker.share_type == ShareType.INTEGER: - fill_quantity = float(int(fill_quantity)) - if not math.isfinite(fill_quantity) or fill_quantity < 0: raise ValueError( "Invalid execution quantity from " f"{type(broker.execution_limits).__name__}: got {fill_quantity!r}" ) + if broker.share_type == ShareType.INTEGER: + fill_quantity = float(int(fill_quantity)) if fill_quantity == 0: return False @@ -176,8 +184,7 @@ def execute(self, order: Order, base_price: float) -> bool: position = broker.positions.get(order.asset) if position is not None: new_qty = position.quantity + signed_qty - is_flip = position.quantity > 0 > new_qty or position.quantity < 0 < new_qty - if is_flip: + if _is_position_flip(position.quantity, new_qty): close_commission = calculate_commission( broker.commission_model, order.asset, @@ -221,20 +228,18 @@ def execute(self, order: Order, base_price: float) -> bool: bid_size=quote_context["bid_size"], ask_size=quote_context["ask_size"], available_size=quote_context["available_size"], - exit_reason=_get_exit_reason(order), + exit_reason=order._exit_reason.value if order._exit_reason is not None else "", exit_reason_detail=order._risk_exit_reason, ) broker.fills.append(fill) # Determine if partial fill is_partial = order.order_id in broker._partial_orders - if is_partial: - order.filled_quantity = (order.filled_quantity or 0) + fill_quantity - else: + order.filled_quantity += fill_quantity + if not is_partial: order.status = OrderStatus.FILLED order.filled_at = current_time order.filled_price = fill_price - order.filled_quantity = fill_quantity # Build fill context ctx = FillContext( @@ -328,7 +333,7 @@ def _update_position(self, ctx: FillContext) -> float: if new_qty == 0: self._close_position(ctx, pos, old_qty) return ctx.commission - elif (old_qty > 0) != (new_qty > 0): + elif _is_position_flip(old_qty, new_qty): return self._flip_position(ctx, pos, old_qty, new_qty) else: self._scale_position(ctx, pos, old_qty, new_qty) @@ -503,8 +508,8 @@ def _flip_position( order = ctx.order # Calculate separate commissions for close and open portions - assert ctx.close_commission is not None - assert ctx.open_commission is not None + if ctx.close_commission is None or ctx.open_commission is None: + raise RuntimeError("Position flip commissions were not calculated before mutation") close_commission = ctx.close_commission open_commission = ctx.open_commission total_commission = close_commission + open_commission @@ -633,6 +638,7 @@ def _scale_position( exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(ctx.order), exit_reason_detail=ctx.order._risk_exit_reason, + status="partial", mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, diff --git a/src/ml4t/backtest/execution/impact.py b/src/ml4t/backtest/execution/impact.py index f555c086..d17db16b 100644 --- a/src/ml4t/backtest/execution/impact.py +++ b/src/ml4t/backtest/execution/impact.py @@ -29,8 +29,9 @@ def calculate( is_buy: True for buy orders, False for sell Returns: - Impact in price units (positive = adverse, negative = favorable) - For buys: price increases; for sells: price decreases + Adverse impact in price units. Values must be finite and non-negative + for buys, or finite and non-positive for sells. Models that represent + price improvement must do so through a separate execution-price model. """ pass diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index f2a0a00d..ee2989ed 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -56,7 +56,8 @@ class BacktestResult: trades: List of completed Trade objects equity_curve: List of (timestamp, portfolio_value) tuples fills: List of Fill objects (all order fills) - rejected_orders: Orders that reached the rejected terminal state + rejected_orders: Orders that reached the rejected terminal state. Orders + cancelled under permissive insufficient-cash handling are not included. predictions: Raw prediction DataFrame passed into the backtest (optional) metrics: Dictionary of computed performance metrics config: BacktestConfig used for the backtest (optional) @@ -106,7 +107,7 @@ def to_trades_dataframe(self) -> pl.DataFrame: quantity, direction, pnl, pnl_percent, bars_held, fees, exit_slippage, mfe, mae, entry_slippage, multiplier, gross_pnl, net_return, total_slippage_cost, cost_drag, - exit_reason, status + exit_reason, exit_reason_detail, status Cost decomposition columns: gross_pnl: Price-move P&L before fees @@ -114,8 +115,8 @@ def to_trades_dataframe(self) -> pl.DataFrame: total_slippage_cost: Entry + exit slippage in dollars cost_drag: Total cost as fraction of notional - The status column indicates "closed" (actually exited) or "open" - (mark-to-market at end of backtest). + The status column indicates "closed" (flat-to-flat completion), "partial" + (realized reduction), or "open" (mark-to-market at end of backtest). Returns: Polars DataFrame with one row per trade @@ -223,6 +224,8 @@ def to_rejected_orders_dataframe(self) -> pl.DataFrame: "symbol": order.asset, "timestamp": order.created_at, "requested_quantity": order.requested_quantity, + "filled_quantity": order.filled_quantity, + "remaining_quantity": order.quantity, "side": order.side.value, "order_type": order.order_type.value, "limit_price": order.limit_price, @@ -787,7 +790,8 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: asset=row["symbol"], created_at=row["timestamp"], requested_quantity=row["requested_quantity"], - quantity=row["requested_quantity"], + quantity=row.get("remaining_quantity", row["requested_quantity"]), + filled_quantity=row.get("filled_quantity", 0.0), side=OrderSide(row["side"]), order_type=OrderType(row["order_type"]), limit_price=row.get("limit_price"), @@ -797,6 +801,7 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: rebalance_id=row.get("rebalance_id"), status=OrderStatus(row["status"]), rejection_reason=row.get("rejection_reason"), + _rejection_code=row.get("rejection_code"), ) ) @@ -909,7 +914,7 @@ def _trades_schema() -> dict[str, pl.DataType]: "cost_drag": pl.Float64(), "exit_reason": pl.String(), "exit_reason_detail": pl.String(), - "status": pl.String(), # "closed" or "open" + "status": pl.String(), # "closed", "partial", or "open" } @staticmethod @@ -949,6 +954,8 @@ def _rejected_orders_schema() -> dict[str, pl.DataType]: "symbol": pl.String(), "timestamp": pl.Datetime(), "requested_quantity": pl.Float64(), + "filled_quantity": pl.Float64(), + "remaining_quantity": pl.Float64(), "side": pl.String(), "order_type": pl.String(), "limit_price": pl.Float64(), diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index fd7657f5..9349bfbb 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -165,6 +165,7 @@ class Order: filled_quantity: float = 0.0 rejection_reason: str | None = None # Reason if order was rejected requested_quantity: float | None = None + _rejection_code: str | None = None # Internal risk management fields (set by broker) _created_bar_index: int = 0 _signal_price: float | None = None # Close price at order creation time @@ -181,6 +182,8 @@ def rejection_code(self) -> str | None: """Return a stable machine-readable category for the rejection reason.""" if self.status is not OrderStatus.REJECTED: return None + if self._rejection_code is not None: + return self._rejection_code reason = (self.rejection_reason or "").lower() if "rounds to zero" in reason: return "quantity_rounds_to_zero" @@ -188,12 +191,12 @@ def rejection_code(self) -> str | None: return "price_unavailable" if "fill check" in reason: return "fill_check_failed" - if "short" in reason or "reversal not allowed" in reason: - return "account_restriction" if "buying power" in reason or "margin" in reason: return "insufficient_buying_power" if "cash" in reason or "insufficient" in reason: return "insufficient_cash" + if "short" in reason or "reversal not allowed" in reason: + return "account_restriction" return "order_validation_failed" @@ -402,13 +405,15 @@ class Fill: @dataclass class Trade: - """Round-trip trade (closed or open). + """Realized exit leg or open position mark. This dataclass is part of the cross-library API specification, designed to produce identical Parquet output across Python, Numba, and Rust implementations. - For open trades (status="open"), exit_time and exit_price represent - mark-to-market values at the end of the backtest period. + Fully closed positions use ``status="closed"``. Incremental reductions use + ``status="partial"`` so lifecycle analytics can exclude repeated position-level + excursion and holding-period values. Open positions use ``status="open"`` and + their exit fields represent end-of-backtest mark-to-market values. Schema Alignment (v0.1.0a6): - symbol: Asset identifier (was 'asset' in earlier versions) @@ -431,7 +436,7 @@ class Trade: # Exit reason for trade analysis (cross-library API field) exit_reason: str = "signal" # ExitReason enum value as string exit_reason_detail: str | None = None - # Trade status: "closed" (actually exited) or "open" (mark-to-market at end) + # Trade status: "closed", "partial", or "open" status: str = "closed" # MFE/MAE preserved from Position for trade analysis (shorter field names) mfe: float = 0.0 # Max favorable excursion (best unrealized return) @@ -512,8 +517,9 @@ class PartialExit: strategies to access trade history during the backtest for stateful decision-making (e.g., adjusting position sizing based on recent wins/losses). - Unlike Trade which represents a fully closed round-trip, PartialExit - captures incremental reductions while the position remains open. + Trade also records partial reductions for result accounting, with + ``status="partial"``. PartialExit is the compact strategy-facing record used + by AssetTradingStats while the position remains open. """ symbol: str # Asset identifier diff --git a/tests/contracts/test_book_parity_behaviors.py b/tests/contracts/test_book_parity_behaviors.py index e5728a32..18033c2b 100644 --- a/tests/contracts/test_book_parity_behaviors.py +++ b/tests/contracts/test_book_parity_behaviors.py @@ -3,7 +3,6 @@ from datetime import datetime, timedelta import polars as pl -import pytest from ml4t.backtest.config import ( BacktestConfig, @@ -89,10 +88,6 @@ def test_snapshot_value_freezes_targets_vs_incremental_recompute() -> None: incremental_result = run_backtest(prices=prices, strategy=incremental_strategy, config=cfg) assert snapshot_strategy.msft_order_qty > incremental_strategy.msft_order_qty - for result in (snapshot_result, incremental_result): - fill_costs = sum(fill.commission for fill in result.fills) - reported_costs = sum(trade.fees for trade in result.trades) - assert reported_costs == pytest.approx(fill_costs, abs=1e-9) class _RotateSellThenBuy(Strategy): diff --git a/tests/helpers/invariants.py b/tests/helpers/invariants.py index bf0c735d..a314ee5c 100644 --- a/tests/helpers/invariants.py +++ b/tests/helpers/invariants.py @@ -43,18 +43,18 @@ def assert_result_invariants( initial_cash: The initial cash used for the backtest. check_*: Flags to selectively disable individual checks. """ - closed_trades = [t for t in result.trades if t.status == "closed"] + realized_trades = [t for t in result.trades if t.status in {"closed", "partial"}] if check_equity_terminal: - _check_equity_terminal(result, initial_cash, closed_trades) + _check_equity_terminal(result, initial_cash, realized_trades) if check_pnl_decomposition: - _check_pnl_decomposition(closed_trades) + _check_pnl_decomposition(realized_trades) if check_direction_signs: - _check_direction_signs(closed_trades) + _check_direction_signs(realized_trades) if check_mfe_mae_bounds: - _check_mfe_mae_bounds(closed_trades) + _check_mfe_mae_bounds(realized_trades) if check_cost_non_negativity: - _check_cost_non_negativity(closed_trades) + _check_cost_non_negativity(realized_trades) if check_fill_temporal_order: _check_fill_temporal_order(result) if check_no_nan: @@ -76,26 +76,34 @@ def _accounting_tolerance(*values: float, operations: int = 1) -> float: def _check_equity_terminal( result: BacktestResult, initial_cash: float, - closed_trades: list, + realized_trades: list, ) -> None: - """Verify: initial_cash + sum(closed_pnl) + sum(open_pnl) ≈ final_value.""" + """Verify: initial_cash + sum(realized_pnl) + sum(open_pnl) ≈ final_value.""" if not result.equity_curve: return final_value = result.equity_curve[-1][1] - closed_pnl = sum(t.pnl for t in closed_trades) + realized_pnl = sum(t.pnl for t in realized_trades) open_trades = [t for t in result.trades if t.status == "open"] open_pnl = sum(t.pnl for t in open_trades) - expected = initial_cash + closed_pnl + open_pnl + expected = initial_cash + realized_pnl + open_pnl diff = abs(expected - final_value) - terms = [initial_cash, *(t.pnl for t in closed_trades), *(t.pnl for t in open_trades)] - tol = _accounting_tolerance(expected, final_value, *terms, operations=len(terms) + 1) + reported_trades = [*realized_trades, *open_trades] + terms = [initial_cash, *(t.pnl for t in reported_trades)] + notionals = [abs(t.quantity) * t.exit_price * t.multiplier for t in reported_trades] + tol = _accounting_tolerance( + expected, + final_value, + *terms, + *notionals, + operations=len(terms) + 1, + ) assert diff <= tol, ( f"Equity terminal invariant violated: " - f"initial_cash({initial_cash}) + closed_pnl({closed_pnl:.6f}) + " + f"initial_cash({initial_cash}) + realized_pnl({realized_pnl:.6f}) + " f"open_pnl({open_pnl:.6f}) = {expected:.6f} != final_value({final_value:.6f}), " f"diff={diff:.10f}, tol={tol:.6f}" ) diff --git a/tests/risk/test_portfolio_manager.py b/tests/risk/test_portfolio_manager.py index 1e214303..3418ae60 100644 --- a/tests/risk/test_portfolio_manager.py +++ b/tests/risk/test_portfolio_manager.py @@ -226,6 +226,7 @@ def test_liquidation_cause_survives_fill_trade_and_artifact( exit_fill = broker.fills[-1] exit_trade = broker.trades[-1] + assert broker.fills[0].exit_reason == "" assert exit_fill.exit_reason == "risk_liquidation" assert exit_fill.exit_reason_detail == detail assert exit_trade.exit_reason == "risk_liquidation" diff --git a/tests/test_broker.py b/tests/test_broker.py index 735229fd..192410cb 100644 --- a/tests/test_broker.py +++ b/tests/test_broker.py @@ -1503,6 +1503,42 @@ def test_evaluate_position_rules_exit_full_immediate(self): assert len(exit_orders) == 1 assert exit_orders[0].quantity == 100.0 assert exit_orders[0]._risk_exit_reason == "stop_loss_5.0%" + broker._process_orders() + assert broker.fills[-1].exit_reason == "stop_loss" + assert broker.fills[-1].exit_reason_detail == "stop_loss_5.0%" + assert broker.trades[-1].exit_reason == "stop_loss" + assert broker.trades[-1].exit_reason_detail == "stop_loss_5.0%" + + def test_partial_rule_exit_preserves_detailed_reason(self): + """Test risk metadata reaches the partial fill and realized leg.""" + from ml4t.backtest.risk.position.dynamic import ScaledExit + + broker = Broker(100000.0, NoCommission(), NoSlippage()) + broker.set_position_rules(ScaledExit([(0.05, 0.25)])) + + mark_prices(broker, {"AAPL": 100.0}) + broker.submit_order("AAPL", 100.0, OrderSide.BUY) + broker._process_orders() + assert broker.fills[0].exit_reason == "" + + broker._update_time( + timestamp=datetime(2024, 1, 2, 9, 30), + prices={"AAPL": 110.0}, + opens={"AAPL": 110.0}, + volumes={"AAPL": 1_000_000}, + highs={"AAPL": 110.0}, + lows={"AAPL": 110.0}, + signals={}, + ) + exit_orders = broker.evaluate_position_rules() + assert len(exit_orders) == 1 + broker._process_orders() + + assert broker.fills[-1].exit_reason == "signal" + assert broker.fills[-1].exit_reason_detail == "scale_out_5%_25%" + assert broker.trades[-1].status == "partial" + assert broker.trades[-1].exit_reason == "signal" + assert broker.trades[-1].exit_reason_detail == "scale_out_5%_25%" def test_evaluate_position_rules_exit_full_deferred(self): """Test EXIT_FULL action with defer_fill=True (NEXT_BAR_OPEN mode).""" diff --git a/tests/test_cost_validation.py b/tests/test_cost_validation.py index 4ab00d4c..687efbb1 100644 --- a/tests/test_cost_validation.py +++ b/tests/test_cost_validation.py @@ -8,7 +8,16 @@ import polars as pl import pytest -from ml4t.backtest import BacktestConfig, DataFeed, Engine, ExecutionMode, Strategy, run_backtest +from ml4t.backtest import ( + BacktestConfig, + DataFeed, + Engine, + ExecutionMode, + OrderSide, + Strategy, + run_backtest, +) +from ml4t.backtest.execution.result import ExecutionResult class _NoOpStrategy(Strategy): @@ -46,6 +55,27 @@ def on_data(self, timestamp, data, context, broker) -> None: self.broker_snapshot = _financial_snapshot(broker) +class _FlipStrategy(Strategy): + def __init__(self, invalid_commission: _ConstantCommission) -> None: + self.invalid_commission = invalid_commission + self.entered = False + self.flip_submitted = False + self.broker_snapshot: dict[str, Any] | None = None + + def on_data(self, timestamp, data, context, broker) -> None: + position = broker.get_position("AAPL") + if not self.entered: + broker.submit_order("AAPL", 10) + self.entered = True + elif position is not None and not self.flip_submitted: + broker.submit_order("AAPL", 20, OrderSide.SELL) + broker.commission_model = self.invalid_commission + broker.gatekeeper.commission_model = self.invalid_commission + broker.skip_cash_validation = True + self.flip_submitted = True + self.broker_snapshot = _financial_snapshot(broker) + + class _ConstantCommission: def __init__(self, value: float) -> None: self.value = value @@ -54,6 +84,17 @@ def calculate(self, asset: str, quantity: float, price: float) -> float: return self.value +class _SnapshotCommission(_ConstantCommission): + def __init__(self, value: float) -> None: + super().__init__(value) + self.broker = None + self.broker_snapshot: dict[str, Any] | None = None + + def calculate(self, asset: str, quantity: float, price: float) -> float: + self.broker_snapshot = _financial_snapshot(self.broker) + return super().calculate(asset, quantity, price) + + class _ConstantSlippage: def __init__(self, value: float) -> None: self.value = value @@ -99,6 +140,23 @@ def calculate( return 0.0 if is_buy else -200.0 +class _InvalidExecutionLimits: + def __init__(self, value: float) -> None: + self.value = value + + def calculate( + self, + order_quantity: float, + bar_volume: float | None, + price: float, + ) -> ExecutionResult: + return ExecutionResult( + fillable_quantity=self.value, + remaining_quantity=order_quantity, + adjusted_price=price, + ) + + def _prices() -> pl.DataFrame: start = datetime(2024, 1, 2) timestamps = [start + timedelta(days=offset) for offset in range(4)] @@ -226,3 +284,56 @@ def test_invalid_execution_price_is_fail_atomic() -> None: assert impact.broker_snapshot is not None assert _financial_snapshot(engine.broker) == impact.broker_snapshot + + +def test_invalid_flip_commission_is_fail_atomic() -> None: + invalid_commission = _SnapshotCommission(math.nan) + strategy = _FlipStrategy(invalid_commission) + engine = Engine( + DataFeed(prices_df=_prices()), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ) + invalid_commission.broker = engine.broker + + with pytest.raises(ValueError, match=r"commission.*_SnapshotCommission"): + engine.run() + + assert invalid_commission.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == invalid_commission.broker_snapshot + + +@pytest.mark.parametrize("value", [-0.5, math.nan, math.inf]) +def test_invalid_execution_limit_quantity_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + limits = _InvalidExecutionLimits(value) + engine = Engine( + DataFeed(prices_df=_prices()), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + execution_limits=limits, + ) + + with pytest.raises(ValueError, match=r"execution quantity.*_InvalidExecutionLimits"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +def test_non_positive_base_execution_price_is_rejected() -> None: + prices = _prices().with_columns( + pl.when(pl.col("timestamp") == pl.col("timestamp").min()) + .then(pl.col("open")) + .otherwise(0.0) + .alias("open") + ) + strategy = _BuyOnceStrategy() + engine = Engine( + DataFeed(prices_df=prices), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ) + + with pytest.raises(ValueError, match=r"base execution price.*0\.0"): + engine.run() diff --git a/tests/test_partial_close_accounting.py b/tests/test_partial_close_accounting.py index 1afa0d70..517785d0 100644 --- a/tests/test_partial_close_accounting.py +++ b/tests/test_partial_close_accounting.py @@ -2,10 +2,18 @@ from datetime import datetime, timedelta +import polars as pl import pytest -from ml4t.backtest import AssetClass, Broker, ContractSpec, OrderSide, OrderStatus -from ml4t.backtest.config import ShareType +from ml4t.backtest import AssetClass, Broker, ContractSpec, OrderSide, OrderStatus, Strategy +from ml4t.backtest.config import ( + BacktestConfig, + CommissionType, + ExecutionMode, + ShareType, + SlippageType, +) +from ml4t.backtest.engine import run_backtest from ml4t.backtest.models import NoSlippage, PercentageCommission @@ -58,6 +66,13 @@ def _execute(broker: Broker, *, day: int, price: float, quantity: float) -> None 1, -5.0, ), + ( + ShareType.INTEGER, + None, + [(100.0, 10.0), (110.0, -4.0), (90.0, 3.0), (120.0, -12.0)], + 2, + -3.0, + ), ( ShareType.INTEGER, {"TEST": ContractSpec("TEST", AssetClass.FUTURE, multiplier=50.0)}, @@ -99,3 +114,43 @@ def test_fill_costs_are_conserved_across_realized_and_residual_positions( for trade in broker.trades: assert trade.gross_pnl - trade.fees == pytest.approx(trade.pnl, abs=1e-9) + + +class _PartialScaleThenClose(Strategy): + def __init__(self) -> None: + self.steps = [10.0, -4.0, 3.0, -9.0] + + def on_data(self, timestamp, data, context, broker) -> None: + quantity = self.steps.pop(0) + side = OrderSide.BUY if quantity > 0 else OrderSide.SELL + broker.submit_order("TEST", abs(quantity), side) + + +def test_partial_exit_scale_up_and_close_reconcile_end_to_end() -> None: + start = datetime(2024, 1, 1) + prices = pl.DataFrame( + { + "timestamp": [start + timedelta(days=day) for day in range(4)], + "asset": ["TEST"] * 4, + "open": [100.0, 110.0, 90.0, 120.0], + "high": [100.0, 110.0, 90.0, 120.0], + "low": [100.0, 110.0, 90.0, 120.0], + "close": [100.0, 110.0, 90.0, 120.0], + "volume": [1_000_000.0] * 4, + } + ) + config = BacktestConfig( + initial_cash=1_000_000.0, + execution_mode=ExecutionMode.SAME_BAR, + commission_type=CommissionType.PERCENTAGE, + commission_rate=0.01, + slippage_type=SlippageType.NONE, + ) + + result = run_backtest(prices, _PartialScaleThenClose(), config=config) + + assert [trade.status for trade in result.trades] == ["partial", "closed"] + assert sum(trade.fees for trade in result.trades) == pytest.approx( + sum(fill.commission for fill in result.fills) + ) + assert result.metrics["num_trades"] == 1 diff --git a/tests/test_rejected_order_results.py b/tests/test_rejected_order_results.py index d763f3eb..6585d2ad 100644 --- a/tests/test_rejected_order_results.py +++ b/tests/test_rejected_order_results.py @@ -3,14 +3,23 @@ from datetime import datetime import polars as pl +import pytest -from ml4t.backtest import BacktestConfig, Strategy, run_backtest -from ml4t.backtest.types import ExecutionMode +from ml4t.backtest import BacktestConfig, Order, Strategy, run_backtest +from ml4t.backtest.config import ShareType +from ml4t.backtest.execution.limits import VolumeParticipationLimit +from ml4t.backtest.types import ExecutionMode, OrderSide, OrderStatus class _UnaffordableOrder(Strategy): + def __init__(self, quantity: float = 1_000_000.0) -> None: + self.quantity = quantity + self.submitted = False + def on_data(self, timestamp, data, context, broker) -> None: - broker.submit_order("AAPL", 1_000_000.0) + if not self.submitted: + broker.submit_order("AAPL", self.quantity) + self.submitted = True class _NoOrders(Strategy): @@ -32,6 +41,20 @@ def _prices() -> pl.DataFrame: ) +def _partial_then_unaffordable_prices() -> pl.DataFrame: + return pl.DataFrame( + { + "timestamp": [datetime(2024, 1, 2), datetime(2024, 1, 3)], + "asset": ["AAPL", "AAPL"], + "open": [50.0, 1_000.0], + "high": [50.0, 1_000.0], + "low": [50.0, 1_000.0], + "close": [50.0, 1_000.0], + "volume": [5.0, 1_000.0], + } + ) + + def _run(strategy: Strategy): return run_backtest( prices=_prices(), @@ -81,6 +104,8 @@ def test_rejected_orders_round_trip_through_result_artifact(tmp_path) -> None: "symbol": "AAPL", "timestamp": datetime(2024, 1, 2), "requested_quantity": 1_000_000.0, + "filled_quantity": 0.0, + "remaining_quantity": 1_000_000.0, "side": "buy", "order_type": "market", "limit_price": None, @@ -97,3 +122,53 @@ def test_rejected_orders_round_trip_through_result_artifact(tmp_path) -> None: result.to_parquet(tmp_path) loaded = type(result).from_parquet(tmp_path) assert loaded.to_rejected_orders_dataframe().to_dicts() == frame.to_dicts() + + loaded.rejected_orders[0].rejection_reason = "Short selling not allowed" + assert loaded.rejected_orders[0].rejection_code == "insufficient_cash" + + +def test_partially_filled_then_rejected_order_is_reconcilable() -> None: + result = run_backtest( + prices=_partial_then_unaffordable_prices(), + strategy=_UnaffordableOrder(15.5), + config=BacktestConfig( + initial_cash=1_000.0, + execution_mode=ExecutionMode.SAME_BAR, + share_type=ShareType.INTEGER, + partial_fills_allowed=True, + ), + execution_limits=VolumeParticipationLimit(max_participation=1.0), + ) + + assert len(result.fills) == 1 + assert len(result.rejected_orders) == 1 + rejected = result.rejected_orders[0] + assert rejected.requested_quantity == 15.5 + assert rejected.filled_quantity == 5.0 + assert rejected.quantity == 10.0 + + record = result.to_rejected_orders_dataframe().to_dicts()[0] + assert record["requested_quantity"] == 15.5 + assert record["filled_quantity"] == 5.0 + assert record["remaining_quantity"] == 10.0 + + +@pytest.mark.parametrize( + ("reason", "expected"), + [ + ("Insufficient cash to cover short", "insufficient_cash"), + ("Insufficient buying power", "insufficient_buying_power"), + ("Short selling not allowed", "account_restriction"), + ("No price available", "price_unavailable"), + ], +) +def test_rejection_code_classification(reason: str, expected: str) -> None: + order = Order( + asset="AAPL", + side=OrderSide.BUY, + quantity=1.0, + status=OrderStatus.REJECTED, + rejection_reason=reason, + ) + + assert order.rejection_code == expected diff --git a/tests/test_result.py b/tests/test_result.py index c8a88c83..16878b20 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -716,6 +716,8 @@ def test_to_parquet_writes_predictions_snapshot(self, sample_predictions: pl.Dat def test_from_parquet_roundtrip(self, backtest_result: BacktestResult): """Test Parquet save and load roundtrip.""" + backtest_result.trades[1].status = "open" + backtest_result.trades[1].exit_reason = "end_of_backtest" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" backtest_result.to_parquet(path) @@ -729,6 +731,7 @@ def test_from_parquet_roundtrip(self, backtest_result: BacktestResult): assert len(loaded.equity_curve) == len(backtest_result.equity_curve) assert len(loaded.portfolio_state) == len(backtest_result.portfolio_state) assert loaded.fills[0].rebalance_id == "rebalance-1" + assert [trade.status for trade in loaded.trades] == ["closed", "open"] assert loaded.metrics["sharpe"] == backtest_result.metrics["sharpe"] def test_to_parquet_compression(self, backtest_result: BacktestResult): From e3201480fbb68243524b5debc32b834b04d5f232 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:21:48 -0400 Subject: [PATCH 07/15] test: require runtime benchmark in CI --- .github/workflows/ci.yml | 2 ++ tests/benchmark/test_hotpath_benchmarks.py | 14 ++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eae7a2e7..dcb5e435 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,6 +77,8 @@ jobs: run: uv sync --dev - name: Run tests + env: + ML4T_REQUIRE_RUNTIME_BENCHMARK: "1" run: uv run pytest tests/ -v --tb=short -x --no-cov contracts: diff --git a/tests/benchmark/test_hotpath_benchmarks.py b/tests/benchmark/test_hotpath_benchmarks.py index 08a234e5..4d9630bd 100644 --- a/tests/benchmark/test_hotpath_benchmarks.py +++ b/tests/benchmark/test_hotpath_benchmarks.py @@ -6,6 +6,7 @@ from __future__ import annotations +import os from datetime import datetime, timedelta from statistics import median from time import perf_counter @@ -158,9 +159,12 @@ def _legacy_view(assets: dict[str, dict[str, Any]]) -> dict[str, dict[str, Any]] } -def _coverage_is_active() -> bool: - """Return whether coverage.py is currently tracing this process.""" - from coverage import Coverage +def _coverage_session_started() -> bool: + """Return whether a coverage.py session has started and not stopped.""" + try: + from coverage import Coverage + except ImportError: + return False return Coverage.current() is not None @@ -183,7 +187,9 @@ def test_optimized_feed_matches_legacy_output(): @pytest.mark.benchmark def test_optimized_feed_runtime_vs_legacy_baseline(): - if _coverage_is_active(): + if _coverage_session_started(): + if os.environ.get("ML4T_REQUIRE_RUNTIME_BENCHMARK") == "1": + pytest.fail("CI requires the runtime benchmark to run without coverage") pytest.skip("Runtime benchmark requires coverage instrumentation to be disabled") prices, signals = _build_benchmark_data(n_bars=3000, n_assets=20) From 6dfc6b06ca639566516d86becad115dea2f0ce0b Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:33:56 -0400 Subject: [PATCH 08/15] feat(results): validate artifact loading Add versioned manifests, strict failure modes, explicit beta recovery, and structured diagnostics for persisted results.\n\nCloses #45. --- docs/user-guide/results.md | 22 + src/ml4t/backtest/__init__.py | 20 +- src/ml4t/backtest/result.py | 543 +++++++++++++++++++------ tests/test_result.py | 150 ++++++- tests/test_trade_cost_decomposition.py | 19 +- 5 files changed, 605 insertions(+), 149 deletions(-) diff --git a/docs/user-guide/results.md b/docs/user-guide/results.md index a8982f2c..c09acd79 100644 --- a/docs/user-guide/results.md +++ b/docs/user-guide/results.md @@ -396,12 +396,34 @@ result.to_parquet("./results/my_backtest") # daily_pnl.parquet # metrics.json # config.yaml # when config is attached +# spec.yaml # when config is attached +# manifest.json # Reload later from ml4t.backtest.result import BacktestResult result = BacktestResult.from_parquet("./results/my_backtest") ``` +`manifest.json` identifies artifact schema version 1 and every component written. Loading is +strict by default. An empty directory, missing manifest or required component, interrupted write, +malformed file, or unsupported schema version raises a specific `ArtifactError` subclass before a +result is returned. Selective exports are valid component exports, but they are not complete result +artifacts unless they contain all required components. + +Manifest-free beta artifacts require explicit recovery: + +```python +result = BacktestResult.from_parquet("./results/beta_backtest", recovery=True) + +for diagnostic in result.artifact_diagnostics: + print(diagnostic.code, diagnostic.component, diagnostic.message) +``` + +Recovery reads supported components in a deterministic order and reports every missing, malformed, +or ignored component. Unsupported manifest schema versions still fail because their interpretation +is not defined. If `config` or `spec` is explicitly requested during export, its absence or a +serialization failure raises `ArtifactWriteError` instead of omitting the file. + ## Integration with ml4t-diagnostic ### Portfolio Analysis (Recommended) diff --git a/src/ml4t/backtest/__init__.py b/src/ml4t/backtest/__init__.py index 553c5696..520783e9 100644 --- a/src/ml4t/backtest/__init__.py +++ b/src/ml4t/backtest/__init__.py @@ -21,7 +21,17 @@ # Execution: rebalancing from .execution.rebalancer import RebalanceConfig, TargetWeightExecutor from .execution.schedule import RebalanceCadence, RebalanceSchedule, resolve_rebalance_timestamps -from .result import BacktestResult +from .result import ( + ArtifactDiagnostic, + ArtifactError, + ArtifactIncompleteError, + ArtifactManifestError, + ArtifactNotFoundError, + ArtifactReadError, + ArtifactWriteError, + BacktestResult, + UnsupportedArtifactVersionError, +) # Risk management rules (position-level) from .risk.position.composite import RuleChain @@ -53,6 +63,14 @@ "run_backtest", "BacktestConfig", "BacktestResult", + "ArtifactDiagnostic", + "ArtifactError", + "ArtifactNotFoundError", + "ArtifactManifestError", + "ArtifactIncompleteError", + "ArtifactReadError", + "ArtifactWriteError", + "UnsupportedArtifactVersionError", "CommissionType", # Canonical domain types "OrderType", diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index ee2989ed..faa6cef8 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -42,6 +42,64 @@ from .config import BacktestConfig +_ARTIFACT_TYPE = "ml4t-backtest-result" +_ARTIFACT_SCHEMA_VERSION = 1 +_MANIFEST_FILE = "manifest.json" +_INCOMPLETE_MARKER = ".artifact-incomplete" +_COMPONENT_FILES = { + "trades": "trades.parquet", + "fills": "fills.parquet", + "rejected_orders": "rejected_orders.parquet", + "predictions": "predictions.parquet", + "equity": "equity.parquet", + "portfolio_state": "portfolio_state.parquet", + "daily_pnl": "daily_pnl.parquet", + "metrics": "metrics.json", + "config": "config.yaml", + "spec": "spec.yaml", +} +_REQUIRED_RESULT_COMPONENTS = frozenset( + {"trades", "fills", "rejected_orders", "equity", "portfolio_state", "daily_pnl", "metrics"} +) + + +@dataclass(frozen=True) +class ArtifactDiagnostic: + """Structured description of one omission or recovery action.""" + + code: str + component: str + message: str + + +class ArtifactError(ValueError): + """Base class for result-artifact failures.""" + + +class ArtifactNotFoundError(ArtifactError): + """Raised when an artifact path does not contain any result data.""" + + +class ArtifactManifestError(ArtifactError): + """Raised when the artifact manifest is missing or malformed.""" + + +class ArtifactIncompleteError(ArtifactError): + """Raised when a current artifact is incomplete.""" + + +class ArtifactReadError(ArtifactError): + """Raised when a declared artifact component cannot be decoded.""" + + +class ArtifactWriteError(ArtifactError): + """Raised when a requested artifact component cannot be written.""" + + +class UnsupportedArtifactVersionError(ArtifactError): + """Raised when an artifact uses an unsupported schema version.""" + + @dataclass class BacktestResult: """Structured backtest result with export capabilities. @@ -77,6 +135,7 @@ class BacktestResult: default_factory=list ) rejected_orders: list[Order] = field(default_factory=list) + artifact_diagnostics: tuple[ArtifactDiagnostic, ...] = field(default_factory=tuple) # Cached DataFrames (computed on demand) _trades_df: pl.DataFrame | None = field(default=None, repr=False) @@ -554,6 +613,7 @@ def to_parquet( {path}/ trades.parquet fills.parquet + rejected_orders.parquet predictions.parquet equity.parquet portfolio_state.parquet @@ -561,6 +621,7 @@ def to_parquet( metrics.json config.yaml (if config available) spec.yaml (if config available) + manifest.json Args: path: Directory path to write files @@ -571,37 +632,47 @@ def to_parquet( Returns: Dict mapping component names to file paths + + Raises: + ArtifactWriteError: If a requested component is unavailable or cannot be written. """ + explicitly_selected = include is not None + requested = list(include) if include is not None else list(_COMPONENT_FILES) + unknown = sorted(set(requested) - _COMPONENT_FILES.keys()) + if unknown: + raise ArtifactWriteError(f"Unknown artifact components requested: {unknown}") + + unavailable: dict[str, str] = {} + if self.predictions is None: + unavailable["predictions"] = "result has no predictions" + if self.config is None: + unavailable["config"] = "result has no config" + unavailable["spec"] = "result has no config for a runtime spec" + + explicitly_unavailable = sorted(set(requested) & unavailable.keys()) + if explicitly_selected and explicitly_unavailable: + details = ", ".join(f"{name}: {unavailable[name]}" for name in explicitly_unavailable) + raise ArtifactWriteError(f"Requested artifact components are unavailable: {details}") + + selected = [name for name in requested if name not in unavailable] path = Path(path) path.mkdir(parents=True, exist_ok=True) - - if include is None: - include = [ - "trades", - "fills", - "rejected_orders", - "predictions", - "equity", - "portfolio_state", - "daily_pnl", - "metrics", - "config", - "spec", - ] + marker_path = path / _INCOMPLETE_MARKER + marker_path.write_text("Result artifact write did not complete.\n") written: dict[str, Path] = {} - if "trades" in include: + if "trades" in selected: trades_path = path / "trades.parquet" self.to_trades_dataframe().write_parquet(trades_path, compression=compression) written["trades"] = trades_path - if "fills" in include: + if "fills" in selected: fills_path = path / "fills.parquet" self.to_fills_dataframe().write_parquet(fills_path, compression=compression) written["fills"] = fills_path - if "rejected_orders" in include: + if "rejected_orders" in selected: rejected_orders_path = path / "rejected_orders.parquet" self.to_rejected_orders_dataframe().write_parquet( rejected_orders_path, @@ -609,29 +680,29 @@ def to_parquet( ) written["rejected_orders"] = rejected_orders_path - if "predictions" in include and self.predictions is not None: + if "predictions" in selected: predictions_path = path / "predictions.parquet" self.to_predictions_dataframe().write_parquet(predictions_path, compression=compression) written["predictions"] = predictions_path - if "equity" in include: + if "equity" in selected: equity_path = path / "equity.parquet" self.to_equity_dataframe().write_parquet(equity_path, compression=compression) written["equity"] = equity_path - if "portfolio_state" in include: + if "portfolio_state" in selected: portfolio_state_path = path / "portfolio_state.parquet" self.to_portfolio_state_dataframe().write_parquet( portfolio_state_path, compression=compression ) written["portfolio_state"] = portfolio_state_path - if "daily_pnl" in include: + if "daily_pnl" in selected: daily_path = path / "daily_pnl.parquet" self.to_daily_pnl().write_parquet(daily_path, compression=compression) written["daily_pnl"] = daily_path - if "metrics" in include: + if "metrics" in selected: metrics_path = path / "metrics.json" # Filter to JSON-serializable metrics serializable = {} @@ -647,58 +718,283 @@ def to_parquet( if isinstance(v, np.generic): serializable[k] = v.item() + continue except (ImportError, AttributeError): - pass # Skip if numpy not available or not a numpy type + pass + raise ArtifactWriteError( + f"Metric {k!r} has unsupported value type {type(v).__name__}" + ) with open(metrics_path, "w") as f: json.dump(serializable, f, indent=2) written["metrics"] = metrics_path - if "config" in include and self.config is not None: + if "config" in selected: config_path = path / "config.yaml" try: import yaml with open(config_path, "w") as f: - yaml.dump(self.config.to_dict(), f, default_flow_style=False) + yaml.safe_dump(self.config.to_dict(), f, default_flow_style=False) written["config"] = config_path - except (ImportError, AttributeError): - pass # Skip if yaml not available or config has no to_dict + except Exception as exc: + raise ArtifactWriteError(f"Failed to write config component: {exc}") from exc - if "spec" in include and self.config is not None: + if "spec" in selected: spec_path = path / "spec.yaml" try: import yaml with open(spec_path, "w") as f: - yaml.dump(self.to_spec_dict(), f, default_flow_style=False, sort_keys=False) + yaml.safe_dump( + self.to_spec_dict(), + f, + default_flow_style=False, + sort_keys=False, + ) written["spec"] = spec_path - except (ImportError, AttributeError): - pass + except Exception as exc: + raise ArtifactWriteError(f"Failed to write spec component: {exc}") from exc + + manifest = { + "artifact_type": _ARTIFACT_TYPE, + "schema_version": _ARTIFACT_SCHEMA_VERSION, + "library_version": __version__, + "complete": written.keys() >= _REQUIRED_RESULT_COMPONENTS, + "components": { + name: _COMPONENT_FILES[name] for name in _COMPONENT_FILES if name in written + }, + "omitted_components": { + name: reason for name, reason in unavailable.items() if name in requested + }, + } + manifest_path = path / _MANIFEST_FILE + try: + with open(manifest_path, "w") as file: + json.dump(manifest, file, indent=2) + except Exception as exc: + raise ArtifactWriteError(f"Failed to write artifact manifest: {exc}") from exc + written["manifest"] = manifest_path + marker_path.unlink() return written @classmethod - def from_parquet(cls, path: str | Path) -> BacktestResult: - """Load backtest result from Parquet directory. + def from_parquet( + cls, + path: str | Path, + *, + recovery: bool = False, + ) -> BacktestResult: + """Load a validated result artifact. Args: - path: Directory containing Parquet files from to_parquet() + path: Directory containing files written by :meth:`to_parquet`. + recovery: Permit manifest-free beta artifacts and omit unreadable components. + Every omission is reported through ``artifact_diagnostics``. Returns: - BacktestResult instance + BacktestResult instance. + + Raises: + ArtifactError: If strict validation or component decoding fails. """ path = Path(path) + artifact_path = path + if not path.exists(): + raise ArtifactNotFoundError(f"Result artifact path does not exist: {path}") + if not path.is_dir(): + raise ArtifactNotFoundError(f"Result artifact path is not a directory: {path}") + + diagnostics: list[ArtifactDiagnostic] = [] + entries = list(path.iterdir()) + if not entries and not recovery: + raise ArtifactNotFoundError(f"Result artifact directory is empty: {path}") + + marker_path = path / _INCOMPLETE_MARKER + if marker_path.exists(): + if not recovery: + raise ArtifactIncompleteError( + f"Result artifact contains {_INCOMPLETE_MARKER}; its write did not complete" + ) + diagnostics.append( + ArtifactDiagnostic( + code="incomplete_write", + component="manifest", + message="Artifact write did not complete.", + ) + ) - # Load trades - trades_path = path / "trades.parquet" - trades: list[Trade] = [] - if trades_path.exists(): - trades_df = pl.read_parquet(trades_path) - for row in trades_df.iter_rows(named=True): - # Support both old (asset/commission) and new (symbol/fees) column names + def discover_legacy_components() -> dict[str, str]: + discovered = { + name: filename + for name, filename in _COMPONENT_FILES.items() + if (artifact_path / filename).exists() + } + if "predictions" not in discovered and (artifact_path / "signals.parquet").exists(): + discovered["predictions"] = "signals.parquet" + return discovered + + manifest_path = path / _MANIFEST_FILE + components: dict[str, str] + manifest: dict[str, Any] | None = None + if not manifest_path.exists(): + if not recovery: + raise ArtifactManifestError( + "Result artifact manifest is missing; pass recovery=True only for retained " + "beta artifacts" + ) + diagnostics.append( + ArtifactDiagnostic( + code="manifest_missing", + component="manifest", + message="Loaded a manifest-free beta artifact.", + ) + ) + components = discover_legacy_components() + else: + try: + with open(manifest_path) as file: + manifest_data = json.load(file) + if not isinstance(manifest_data, dict): + raise TypeError("manifest root must be an object") + manifest = manifest_data + except Exception as exc: + if not recovery: + raise ArtifactManifestError( + f"Failed to read {_MANIFEST_FILE}: {type(exc).__name__}: {exc}" + ) from exc + diagnostics.append( + ArtifactDiagnostic( + code="manifest_invalid", + component="manifest", + message=f"Ignored malformed manifest ({type(exc).__name__}).", + ) + ) + components = discover_legacy_components() + + if manifest is not None: + artifact_type = manifest.get("artifact_type") + if artifact_type != _ARTIFACT_TYPE: + message = f"Unsupported artifact type: {artifact_type!r}" + if not recovery: + raise ArtifactManifestError(message) + diagnostics.append(ArtifactDiagnostic("manifest_invalid", "manifest", message)) + components = discover_legacy_components() + manifest = None + + if manifest is not None: + schema_version = manifest.get("schema_version") + if schema_version != _ARTIFACT_SCHEMA_VERSION: + raise UnsupportedArtifactVersionError( + f"Unsupported result artifact schema version {schema_version!r}; " + f"supported version is {_ARTIFACT_SCHEMA_VERSION}" + ) + component_data = manifest.get("components") + if not isinstance(component_data, dict) or not all( + isinstance(name, str) and isinstance(filename, str) + for name, filename in component_data.items() + ): + if not recovery: + raise ArtifactManifestError("Manifest components must be a string mapping") + diagnostics.append( + ArtifactDiagnostic( + "manifest_invalid", + "manifest", + "Ignored invalid component mapping.", + ) + ) + components = discover_legacy_components() + else: + unknown = sorted(set(component_data) - _COMPONENT_FILES.keys()) + noncanonical = sorted( + name + for name, filename in component_data.items() + if name in _COMPONENT_FILES and filename != _COMPONENT_FILES[name] + ) + if unknown or noncanonical: + details = f"unknown={unknown}, noncanonical={noncanonical}" + if not recovery: + raise ArtifactManifestError(f"Invalid manifest components: {details}") + diagnostics.append( + ArtifactDiagnostic( + "manifest_invalid", + "manifest", + f"Ignored invalid manifest components: {details}.", + ) + ) + components = discover_legacy_components() + else: + components = dict(component_data) + + declared_incomplete = manifest is not None and manifest.get("complete") is not True + if declared_incomplete and not recovery: + raise ArtifactIncompleteError("Result artifact manifest marks the export incomplete") + if declared_incomplete: + diagnostics.append( + ArtifactDiagnostic( + code="manifest_incomplete", + component="manifest", + message="Manifest marks this as a selective or incomplete export.", + ) + ) + + missing_required_components = sorted(_REQUIRED_RESULT_COMPONENTS - components.keys()) + missing_files = sorted( + name for name, filename in components.items() if not (path / filename).is_file() + ) + if not recovery and (missing_required_components or missing_files): + raise ArtifactIncompleteError( + "Result artifact is incomplete: " + f"missing components={missing_required_components}, missing files={missing_files}" + ) + if recovery: + missing_components = sorted(_COMPONENT_FILES.keys() - components.keys()) + diagnostics.extend( + ArtifactDiagnostic( + code="component_missing", + component=name, + message=f"Component {_COMPONENT_FILES[name]} is absent.", + ) + for name in missing_components + ) + for name in missing_files: + diagnostics.append( + ArtifactDiagnostic( + code="component_missing_file", + component=name, + message=f"Declared component {components[name]} is absent.", + ) + ) + components.pop(name) + + def read_component(name: str, reader, default): + filename = components.get(name) + if filename is None: + return default + try: + return reader(path / filename) + except Exception as exc: + if not recovery: + raise ArtifactReadError( + f"Failed to read {filename}: {type(exc).__name__}: {exc}" + ) from exc + diagnostics.append( + ArtifactDiagnostic( + code="component_invalid", + component=name, + message=f"Ignored unreadable {filename} ({type(exc).__name__}).", + ) + ) + return default + + def read_trades(component_path: Path) -> list[Trade]: + result: list[Trade] = [] + for row in pl.read_parquet(component_path).iter_rows(named=True): symbol = row.get("symbol") or row.get("asset", "") - fees = row.get("fees") or row.get("commission", 0.0) - trades.append( + fees = row.get("fees") + if fees is None: + fees = row.get("commission", 0.0) + result.append( Trade( symbol=symbol, entry_time=row["entry_time"], @@ -714,8 +1010,8 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: exit_reason=row.get("exit_reason", "signal"), exit_reason_detail=row.get("exit_reason_detail"), status=row.get("status", "closed"), - mfe=row["mfe"], - mae=row["mae"], + mfe=row.get("mfe", 0.0), + mae=row.get("mae", 0.0), entry_slippage=row.get("entry_slippage", 0.0), multiplier=row.get("multiplier", 1.0), entry_quote_mid_price=row.get("entry_quote_mid_price"), @@ -730,28 +1026,12 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: exit_available_size=row.get("exit_available_size"), ) ) + return result - # Load equity curve - equity_curve: list[tuple[datetime, float]] = [] - equity_path = path / "equity.parquet" - if equity_path.exists(): - equity_df = pl.read_parquet(equity_path) - for row in equity_df.iter_rows(named=True): - equity_curve.append((row["timestamp"], row["equity"])) - - # Load metrics - metrics: dict[str, Any] = {} - metrics_path = path / "metrics.json" - if metrics_path.exists(): - with open(metrics_path) as f: - metrics = json.load(f) - - fills: list[Fill] = [] - fills_path = path / "fills.parquet" - if fills_path.exists(): - fills_df = pl.read_parquet(fills_path) - for row in fills_df.iter_rows(named=True): - fills.append( + def read_fills(component_path: Path) -> list[Fill]: + result: list[Fill] = [] + for row in pl.read_parquet(component_path).iter_rows(named=True): + result.append( Fill( order_id=row["order_id"], rebalance_id=row.get("rebalance_id"), @@ -778,13 +1058,12 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: exit_reason_detail=row.get("exit_reason_detail"), ) ) + return result - rejected_orders: list[Order] = [] - rejected_orders_path = path / "rejected_orders.parquet" - if rejected_orders_path.exists(): - rejected_orders_df = pl.read_parquet(rejected_orders_path) - for row in rejected_orders_df.iter_rows(named=True): - rejected_orders.append( + def read_rejected_orders(component_path: Path) -> list[Order]: + result: list[Order] = [] + for row in pl.read_parquet(component_path).iter_rows(named=True): + result.append( Order( order_id=row["order_id"], asset=row["symbol"], @@ -804,60 +1083,75 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: _rejection_code=row.get("rejection_code"), ) ) + return result - predictions = None - predictions_path = path / "predictions.parquet" - if predictions_path.exists(): - predictions = pl.read_parquet(predictions_path) - else: - signals_path = path / "signals.parquet" - if signals_path.exists(): - predictions = pl.read_parquet(signals_path) - - portfolio_state: list[tuple[datetime, float, float, float, float, int]] = [] - portfolio_state_path = path / "portfolio_state.parquet" - if portfolio_state_path.exists(): - portfolio_state_df = pl.read_parquet(portfolio_state_path) - for row in portfolio_state_df.iter_rows(named=True): - portfolio_state.append( - ( - row["timestamp"], - row["equity"], - row["cash"], - row["gross_exposure"], - row["net_exposure"], - row["open_positions"], - ) - ) - - # Load config if available - config = None - config_path = path / "config.yaml" - if config_path.exists(): - try: - import yaml - - from .config import BacktestConfig - - with open(config_path) as f: - config_data = yaml.safe_load(f) - config = BacktestConfig.from_dict(config_data) - except (ImportError, Exception): - pass # Skip if yaml not available or config invalid - else: - spec_path = path / "spec.yaml" - if spec_path.exists(): - try: - import yaml + def read_equity(component_path: Path) -> list[tuple[datetime, float]]: + return [ + (row["timestamp"], row["equity"]) + for row in pl.read_parquet(component_path).iter_rows(named=True) + ] - from .config import BacktestConfig + def read_portfolio_state( + component_path: Path, + ) -> list[tuple[datetime, float, float, float, float, int]]: + return [ + ( + row["timestamp"], + row["equity"], + row["cash"], + row["gross_exposure"], + row["net_exposure"], + row["open_positions"], + ) + for row in pl.read_parquet(component_path).iter_rows(named=True) + ] - with open(spec_path) as f: - spec_data = yaml.safe_load(f) - if isinstance(spec_data, dict) and isinstance(spec_data.get("config"), dict): - config = BacktestConfig.from_dict(spec_data["config"]) - except (ImportError, Exception): - pass + def read_metrics(component_path: Path) -> dict[str, Any]: + with open(component_path) as file: + data = json.load(file) + if not isinstance(data, dict): + raise TypeError("metrics root must be an object") + return data + + def read_config(component_path: Path): + import yaml + + from .config import BacktestConfig + + with open(component_path) as file: + data = yaml.safe_load(file) + if not isinstance(data, dict): + raise TypeError("config root must be a mapping") + return BacktestConfig.from_dict(data) + + def read_spec_config(component_path: Path): + import yaml + + from .config import BacktestConfig + + with open(component_path) as file: + data = yaml.safe_load(file) + if not isinstance(data, dict): + raise TypeError("spec root must be a mapping") + if data.get("version") != 1: + raise ValueError(f"unsupported spec version {data.get('version')!r}") + config_data = data.get("config") + if not isinstance(config_data, dict): + raise TypeError("spec config must be a mapping") + return BacktestConfig.from_dict(config_data) + + trades = read_component("trades", read_trades, []) + fills = read_component("fills", read_fills, []) + rejected_orders = read_component("rejected_orders", read_rejected_orders, []) + equity_curve = read_component("equity", read_equity, []) + portfolio_state = read_component("portfolio_state", read_portfolio_state, []) + metrics = read_component("metrics", read_metrics, {}) + predictions = read_component("predictions", pl.read_parquet, None) + read_component("daily_pnl", pl.read_parquet, None) + config = read_component("config", read_config, None) + spec_config = read_component("spec", read_spec_config, None) + if config is None: + config = spec_config return cls( trades=trades, @@ -868,6 +1162,7 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: rejected_orders=rejected_orders, metrics=metrics, config=config, + artifact_diagnostics=tuple(diagnostics), ) @staticmethod diff --git a/tests/test_result.py b/tests/test_result.py index 16878b20..ea7946bf 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -14,7 +14,13 @@ from ml4t.backtest.config import BacktestConfig from ml4t.backtest.result import ( + ArtifactIncompleteError, + ArtifactManifestError, + ArtifactNotFoundError, + ArtifactReadError, + ArtifactWriteError, BacktestResult, + UnsupportedArtifactVersionError, enrich_trades_with_signals, ) from ml4t.backtest.types import Fill, OrderSide, Trade @@ -646,8 +652,9 @@ def test_to_parquet_selective(self, backtest_result: BacktestResult): assert "equity" not in written assert "portfolio_state" not in written - def test_to_parquet_config_write_failure_is_non_fatal(self): - """Test config export failure is swallowed (ImportError/AttributeError path).""" + @pytest.mark.parametrize("component", ["config", "spec"]) + def test_to_parquet_config_write_failure_is_explicit(self, component: str): + """Test requested config and spec failures identify the component.""" class _BadConfig: def to_dict(self): @@ -662,8 +669,20 @@ def to_dict(self): ) with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" - written = result.to_parquet(path, include=["config"]) - assert "config" not in written + with pytest.raises(ArtifactWriteError, match=component): + result.to_parquet(path, include=[component]) + with pytest.raises(ArtifactIncompleteError, match="did not complete"): + BacktestResult.from_parquet(path) + + @pytest.mark.parametrize("component", ["config", "spec"]) + def test_to_parquet_rejects_requested_config_without_config(self, component: str): + result = BacktestResult(trades=[], equity_curve=[], fills=[], metrics={}) + + with ( + tempfile.TemporaryDirectory() as tmpdir, + pytest.raises(ArtifactWriteError, match=component), + ): + result.to_parquet(Path(tmpdir) / "test_backtest", include=[component]) def test_to_parquet_writes_spec_snapshot(self): """Test resolved runtime spec export.""" @@ -733,6 +752,21 @@ def test_from_parquet_roundtrip(self, backtest_result: BacktestResult): assert loaded.fills[0].rebalance_id == "rebalance-1" assert [trade.status for trade in loaded.trades] == ["closed", "open"] assert loaded.metrics["sharpe"] == backtest_result.metrics["sharpe"] + assert loaded.artifact_diagnostics == () + + with open(path / "manifest.json") as file: + manifest = json.load(file) + assert manifest["artifact_type"] == "ml4t-backtest-result" + assert manifest["schema_version"] == 1 + assert set(manifest["components"]) >= { + "trades", + "fills", + "rejected_orders", + "equity", + "portfolio_state", + "daily_pnl", + "metrics", + } def test_to_parquet_compression(self, backtest_result: BacktestResult): """Test different compression codecs.""" @@ -743,27 +777,84 @@ def test_to_parquet_compression(self, backtest_result: BacktestResult): assert written["trades"].exists() def test_from_parquet_empty_dir(self): - """Test loading from directory without files.""" + """Test empty directories fail before returning an empty result.""" + with ( + tempfile.TemporaryDirectory() as tmpdir, + pytest.raises(ArtifactNotFoundError, match="empty"), + ): + BacktestResult.from_parquet(tmpdir) + + def test_from_parquet_invalid_config_fails_strict_and_reports_recovery(self): with tempfile.TemporaryDirectory() as tmpdir: - loaded = BacktestResult.from_parquet(tmpdir) - assert len(loaded.trades) == 0 - assert len(loaded.equity_curve) == 0 + path = Path(tmpdir) / "test_backtest" + result = BacktestResult( + trades=[], + equity_curve=[], + fills=[], + metrics={}, + config=BacktestConfig(), + ) + result.to_parquet(path) + (path / "config.yaml").write_text("bad: [") + + with pytest.raises(ArtifactReadError, match="config.yaml"): + BacktestResult.from_parquet(path) - def test_from_parquet_invalid_config_is_non_fatal(self, monkeypatch): - """Test config load failures are swallowed and config remains None.""" + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.config is not None # Recovered from the valid spec component. + assert [(item.code, item.component) for item in recovered.artifact_diagnostics] == [ + ("component_missing", "predictions"), + ("component_invalid", "config"), + ] + + def test_from_parquet_rejects_missing_required_component(self, backtest_result: BacktestResult): with tempfile.TemporaryDirectory() as tmpdir: - path = Path(tmpdir) - (path / "config.yaml").write_text("bad: [") - # Force yaml.safe_load failure branch - import yaml + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / "fills.parquet").unlink() - monkeypatch.setattr( - yaml, - "safe_load", - lambda *_args, **_kwargs: (_ for _ in ()).throw(ValueError("bad yaml")), - ) - loaded = BacktestResult.from_parquet(path) - assert loaded.config is None + with pytest.raises(ArtifactIncompleteError, match="fills"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert [(item.code, item.component) for item in recovered.artifact_diagnostics] == [ + ("component_missing", "config"), + ("component_missing", "spec"), + ("component_missing_file", "fills"), + ] + + def test_from_parquet_rejects_corrupt_metrics(self, backtest_result: BacktestResult): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / "metrics.json").write_text("{") + + with pytest.raises(ArtifactReadError, match="metrics.json"): + BacktestResult.from_parquet(path) + + def test_from_parquet_rejects_unsupported_schema(self, backtest_result: BacktestResult): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + manifest_path = path / "manifest.json" + manifest = json.loads(manifest_path.read_text()) + manifest["schema_version"] = 999 + manifest_path.write_text(json.dumps(manifest)) + + with pytest.raises(UnsupportedArtifactVersionError, match="999"): + BacktestResult.from_parquet(path) + + def test_from_parquet_rejects_malformed_manifest(self, backtest_result: BacktestResult): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / "manifest.json").write_text("{") + + with pytest.raises(ArtifactManifestError, match="manifest.json"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.artifact_diagnostics[0].code == "manifest_invalid" def test_from_parquet_loads_config_from_spec_when_config_yaml_missing(self): """Test spec.yaml fallback restores replayable config.""" @@ -783,11 +874,12 @@ def test_from_parquet_loads_config_from_spec_when_config_yaml_missing(self): path = Path(tmpdir) / "test_backtest" result.to_parquet(path, include=["spec"]) - loaded = BacktestResult.from_parquet(path) + loaded = BacktestResult.from_parquet(path, recovery=True) assert loaded.config is not None assert loaded.config.initial_cash == 82000.0 assert loaded.config.metadata["strategy_id"] == "spec_fallback" + assert any(item.code == "component_missing" for item in loaded.artifact_diagnostics) def test_metrics_json_serialization(self, backtest_result: BacktestResult): """Test metrics JSON contains only serializable values.""" @@ -801,6 +893,20 @@ def test_metrics_json_serialization(self, backtest_result: BacktestResult): assert isinstance(metrics["sharpe"], float) assert isinstance(metrics["final_value"], float) + def test_metrics_json_rejects_unserializable_values(self): + result = BacktestResult( + trades=[], + equity_curve=[], + fills=[], + metrics={"opaque": object()}, + ) + + with ( + tempfile.TemporaryDirectory() as tmpdir, + pytest.raises(ArtifactWriteError, match="opaque"), + ): + result.to_parquet(Path(tmpdir) / "test_backtest") + class TestEnrichTradesWithSignals: """Tests for enrich_trades_with_signals().""" diff --git a/tests/test_trade_cost_decomposition.py b/tests/test_trade_cost_decomposition.py index c66aca96..c234cc48 100644 --- a/tests/test_trade_cost_decomposition.py +++ b/tests/test_trade_cost_decomposition.py @@ -424,15 +424,30 @@ def test_backward_compat_missing_fields(self, tmp_path): result_dir.mkdir() old_df.write_parquet(result_dir / "trades.parquet") - from ml4t.backtest.result import BacktestResult + from ml4t.backtest.result import ArtifactManifestError, BacktestResult + + with pytest.raises(ArtifactManifestError, match="manifest"): + BacktestResult.from_parquet(result_dir) - loaded = BacktestResult.from_parquet(result_dir) + loaded = BacktestResult.from_parquet(result_dir, recovery=True) assert len(loaded.trades) == 1 t = loaded.trades[0] assert t.exit_slippage == pytest.approx(0.12) # Legacy slippage column maps through assert t.entry_slippage == 0.0 # Default assert t.multiplier == 1.0 # Default assert t.gross_pnl == pytest.approx(1000.0) + assert [(item.code, item.component) for item in loaded.artifact_diagnostics] == [ + ("manifest_missing", "manifest"), + ("component_missing", "config"), + ("component_missing", "daily_pnl"), + ("component_missing", "equity"), + ("component_missing", "fills"), + ("component_missing", "metrics"), + ("component_missing", "portfolio_state"), + ("component_missing", "predictions"), + ("component_missing", "rejected_orders"), + ("component_missing", "spec"), + ] # === Integration: actual backtest with shorts === From c802b8c2b33e4b72558806904990c7d02f8da1a7 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:46:00 -0400 Subject: [PATCH 09/15] fix(results): harden artifact serialization --- docs/user-guide/results.md | 5 +- src/ml4t/backtest/export.py | 9 ++- src/ml4t/backtest/result.py | 125 +++++++++++++++++++++++------------- tests/test_export.py | 11 ++++ tests/test_result.py | 57 +++++++++++++++- 5 files changed, 155 insertions(+), 52 deletions(-) diff --git a/docs/user-guide/results.md b/docs/user-guide/results.md index c09acd79..128839cd 100644 --- a/docs/user-guide/results.md +++ b/docs/user-guide/results.md @@ -421,8 +421,9 @@ for diagnostic in result.artifact_diagnostics: Recovery reads supported components in a deterministic order and reports every missing, malformed, or ignored component. Unsupported manifest schema versions still fail because their interpretation -is not defined. If `config` or `spec` is explicitly requested during export, its absence or a -serialization failure raises `ArtifactWriteError` instead of omitting the file. +is not defined. If an export selects `config` or `spec`, whether through the default component set +or an explicit `include`, its absence or a serialization failure raises `ArtifactWriteError` +instead of omitting the file. Component payloads are serialized before output files are created. ## Integration with ml4t-diagnostic diff --git a/src/ml4t/backtest/export.py b/src/ml4t/backtest/export.py index 19fc1894..60eb7f8b 100644 --- a/src/ml4t/backtest/export.py +++ b/src/ml4t/backtest/export.py @@ -161,20 +161,25 @@ def batch_export( return summary_df @staticmethod - def from_parquet(path: str | Path) -> BacktestResult: + def from_parquet(path: str | Path, *, recovery: bool = False) -> BacktestResult: """Load backtest result from Parquet directory. Delegates to BacktestResult.from_parquet(). Args: path: Directory containing Parquet files + recovery: Permit manifest-free beta artifacts and report omissions on + the returned result. Returns: BacktestResult instance + + Raises: + ArtifactError: If strict artifact validation or decoding fails. """ from .result import BacktestResult - return BacktestResult.from_parquet(path) + return BacktestResult.from_parquet(path, recovery=recovery) @staticmethod def load_sweep_summary(base_path: str | Path) -> pl.DataFrame: diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index faa6cef8..2cec13e3 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -63,6 +63,33 @@ ) +def _serialize_metric_value(value: Any, *, path: str) -> Any: + """Convert a metric value to JSON-safe built-in containers and scalars.""" + if isinstance(value, int | float | str | bool | type(None)): + return value + if isinstance(value, datetime): + return value.isoformat() + if isinstance(value, list | tuple): + return [ + _serialize_metric_value(item, path=f"{path}[{index}]") + for index, item in enumerate(value) + ] + if isinstance(value, dict): + if not all(isinstance(key, str) for key in value): + raise ArtifactWriteError(f"{path} contains a non-string mapping key") + return { + key: _serialize_metric_value(item, path=f"{path}.{key}") for key, item in value.items() + } + try: + import numpy as np + + if isinstance(value, np.generic): + return _serialize_metric_value(value.item(), path=path) + except (ImportError, AttributeError): + pass + raise ArtifactWriteError(f"{path} has unsupported value type {type(value).__name__}") + + @dataclass(frozen=True) class ArtifactDiagnostic: """Structured description of one omission or recovery action.""" @@ -121,6 +148,8 @@ class BacktestResult: config: BacktestConfig used for the backtest (optional) equity: EquityCurve analytics object trade_analyzer: TradeAnalyzer analytics object + artifact_diagnostics: Structured omissions and recovery actions. Empty for + artifacts loaded successfully in strict mode. """ trades: list[Trade] @@ -631,7 +660,9 @@ def to_parquet( compression: Parquet compression codec (default: "zstd") Returns: - Dict mapping component names to file paths + Dict mapping requested component names to file paths. The always-written + manifest is returned under the additional ``"manifest"`` key; it is not + a selectable component. Raises: ArtifactWriteError: If a requested component is unavailable or cannot be written. @@ -655,6 +686,43 @@ def to_parquet( raise ArtifactWriteError(f"Requested artifact components are unavailable: {details}") selected = [name for name in requested if name not in unavailable] + + metrics_payload: str | None = None + if "metrics" in selected: + try: + serializable_metrics = { + key: _serialize_metric_value(value, path=f"metrics[{key!r}]") + for key, value in self.metrics.items() + } + metrics_payload = json.dumps(serializable_metrics, indent=2) + except ArtifactWriteError: + raise + except Exception as exc: + raise ArtifactWriteError(f"Failed to serialize metrics: {exc}") from exc + + config_payload: str | None = None + spec_payload: str | None = None + if "config" in selected or "spec" in selected: + try: + import yaml + + if "config" in selected: + config_payload = yaml.safe_dump( + self.config.to_dict(), + default_flow_style=False, + ) + if "spec" in selected: + spec_payload = yaml.safe_dump( + self.to_spec_dict(), + default_flow_style=False, + sort_keys=False, + ) + except Exception as exc: + components = [name for name in ("config", "spec") if name in selected] + raise ArtifactWriteError( + f"Failed to serialize {' and '.join(components)} component: {exc}" + ) from exc + path = Path(path) path.mkdir(parents=True, exist_ok=True) marker_path = path / _INCOMPLETE_MARKER @@ -704,56 +772,21 @@ def to_parquet( if "metrics" in selected: metrics_path = path / "metrics.json" - # Filter to JSON-serializable metrics - serializable = {} - for k, v in self.metrics.items(): - if isinstance(v, int | float | str | bool | type(None)): - serializable[k] = v - elif isinstance(v, datetime): - serializable[k] = v.isoformat() - else: - # Handle numpy scalars (np.float64, np.int64, etc.) - try: - import numpy as np - - if isinstance(v, np.generic): - serializable[k] = v.item() - continue - except (ImportError, AttributeError): - pass - raise ArtifactWriteError( - f"Metric {k!r} has unsupported value type {type(v).__name__}" - ) - with open(metrics_path, "w") as f: - json.dump(serializable, f, indent=2) + assert metrics_payload is not None + metrics_path.write_text(metrics_payload) written["metrics"] = metrics_path if "config" in selected: config_path = path / "config.yaml" - try: - import yaml - - with open(config_path, "w") as f: - yaml.safe_dump(self.config.to_dict(), f, default_flow_style=False) - written["config"] = config_path - except Exception as exc: - raise ArtifactWriteError(f"Failed to write config component: {exc}") from exc + assert config_payload is not None + config_path.write_text(config_payload) + written["config"] = config_path if "spec" in selected: spec_path = path / "spec.yaml" - try: - import yaml - - with open(spec_path, "w") as f: - yaml.safe_dump( - self.to_spec_dict(), - f, - default_flow_style=False, - sort_keys=False, - ) - written["spec"] = spec_path - except Exception as exc: - raise ArtifactWriteError(f"Failed to write spec component: {exc}") from exc + assert spec_payload is not None + spec_path.write_text(spec_payload) + written["spec"] = spec_path manifest = { "artifact_type": _ARTIFACT_TYPE, @@ -835,7 +868,7 @@ def discover_legacy_components() -> dict[str, str]: return discovered manifest_path = path / _MANIFEST_FILE - components: dict[str, str] + components: dict[str, str] = {} manifest: dict[str, Any] | None = None if not manifest_path.exists(): if not recovery: @@ -1147,7 +1180,7 @@ def read_spec_config(component_path: Path): portfolio_state = read_component("portfolio_state", read_portfolio_state, []) metrics = read_component("metrics", read_metrics, {}) predictions = read_component("predictions", pl.read_parquet, None) - read_component("daily_pnl", pl.read_parquet, None) + read_component("daily_pnl", lambda value: pl.scan_parquet(value).collect_schema(), None) config = read_component("config", read_config, None) spec_config = read_component("spec", read_spec_config, None) if config is None: diff --git a/tests/test_export.py b/tests/test_export.py index 69d0defa..72e128f2 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -144,6 +144,17 @@ def test_from_parquet_delegation(self, sample_result: BacktestResult): assert len(loaded.trades) == 1 assert loaded.metrics["sharpe"] == 1.5 + def test_from_parquet_passes_through_beta_recovery(self, sample_result: BacktestResult): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_export" + sample_result.to_parquet(path) + (path / "manifest.json").unlink() + + loaded = BacktestExporter.from_parquet(path, recovery=True) + + assert len(loaded.trades) == 1 + assert loaded.artifact_diagnostics[0].code == "manifest_missing" + class TestBacktestExporterBatch: """Tests for batch export functionality.""" diff --git a/tests/test_result.py b/tests/test_result.py index ea7946bf..8d300956 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -651,6 +651,36 @@ def test_to_parquet_selective(self, backtest_result: BacktestResult): assert "predictions" not in written assert "equity" not in written assert "portfolio_state" not in written + with pytest.raises(ArtifactIncompleteError, match="marks the export incomplete"): + BacktestResult.from_parquet(path) + + def test_default_manifest_records_unavailable_optional_components(self): + result = BacktestResult(trades=[], equity_curve=[], fills=[], metrics={}) + + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + result.to_parquet(path) + manifest = json.loads((path / "manifest.json").read_text()) + + assert manifest["omitted_components"] == { + "predictions": "result has no predictions", + "config": "result has no config", + "spec": "result has no config for a runtime spec", + } + + def test_incomplete_write_marker_fails_strict_and_reports_recovery( + self, backtest_result: BacktestResult + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / ".artifact-incomplete").write_text("interrupted\n") + + with pytest.raises(ArtifactIncompleteError, match="did not complete"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.artifact_diagnostics[0].code == "incomplete_write" @pytest.mark.parametrize("component", ["config", "spec"]) def test_to_parquet_config_write_failure_is_explicit(self, component: str): @@ -671,8 +701,7 @@ def to_dict(self): path = Path(tmpdir) / "test_backtest" with pytest.raises(ArtifactWriteError, match=component): result.to_parquet(path, include=[component]) - with pytest.raises(ArtifactIncompleteError, match="did not complete"): - BacktestResult.from_parquet(path) + assert not path.exists() @pytest.mark.parametrize("component", ["config", "spec"]) def test_to_parquet_rejects_requested_config_without_config(self, component: str): @@ -856,6 +885,26 @@ def test_from_parquet_rejects_malformed_manifest(self, backtest_result: Backtest recovered = BacktestResult.from_parquet(path, recovery=True) assert recovered.artifact_diagnostics[0].code == "manifest_invalid" + def test_legacy_signals_component_loads_only_in_recovery(self): + predictions = pl.DataFrame( + { + "timestamp": [datetime(2024, 1, 1)], + "asset": ["AAPL"], + "score": [0.75], + } + ) + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "beta_result" + path.mkdir() + predictions.write_parquet(path / "signals.parquet") + + with pytest.raises(ArtifactManifestError, match="manifest"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.predictions is not None + assert recovered.predictions.equals(predictions) + def test_from_parquet_loads_config_from_spec_when_config_yaml_missing(self): """Test spec.yaml fallback restores replayable config.""" config = BacktestConfig( @@ -883,6 +932,8 @@ def test_from_parquet_loads_config_from_spec_when_config_yaml_missing(self): def test_metrics_json_serialization(self, backtest_result: BacktestResult): """Test metrics JSON contains only serializable values.""" + backtest_result.metrics["monthly_returns"] = [0.01, -0.02] + backtest_result.metrics["segments"] = {"train": (1, 2), "test": [3]} with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" backtest_result.to_parquet(path) @@ -892,6 +943,8 @@ def test_metrics_json_serialization(self, backtest_result: BacktestResult): assert isinstance(metrics["sharpe"], float) assert isinstance(metrics["final_value"], float) + assert metrics["monthly_returns"] == [0.01, -0.02] + assert metrics["segments"] == {"train": [1, 2], "test": [3]} def test_metrics_json_rejects_unserializable_values(self): result = BacktestResult( From 7a58a04601967ddb4ac754b6fa6e8c4a753a9b3c Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:56:04 -0400 Subject: [PATCH 10/15] fix: close milestone review regressions --- .github/workflows/ci.yml | 8 +- README.md | 6 ++ docs/user-guide/results.md | 11 +- src/ml4t/backtest/accounting/gatekeeper.py | 17 ++++ src/ml4t/backtest/analytics/trades.py | 24 +++-- src/ml4t/backtest/core/execution_engine.py | 102 +++++++++++-------- src/ml4t/backtest/core/order_book.py | 48 ++++++--- src/ml4t/backtest/engine.py | 7 +- src/ml4t/backtest/execution/fill_executor.py | 12 ++- src/ml4t/backtest/result.py | 59 ++++++++--- src/ml4t/backtest/types.py | 8 ++ tests/benchmark/test_hotpath_benchmarks.py | 5 +- tests/test_broker.py | 14 +++ tests/test_partial_close_accounting.py | 6 +- tests/test_rejected_order_results.py | 66 +++++++++++- tests/test_result.py | 96 +++++++++++++++++ 16 files changed, 395 insertions(+), 94 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcb5e435..914ea575 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,10 +77,14 @@ jobs: run: uv sync --dev - name: Run tests - env: - ML4T_REQUIRE_RUNTIME_BENCHMARK: "1" run: uv run pytest tests/ -v --tb=short -x --no-cov + - name: Run runtime regression benchmark + run: >- + uv run pytest + tests/benchmark/test_hotpath_benchmarks.py::test_optimized_feed_runtime_vs_legacy_baseline + -v --tb=short --no-cov + contracts: name: Cross-Engine Contracts runs-on: ubuntu-latest diff --git a/README.md b/README.md index 61624f7b..d95c7aa6 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,12 @@ python validation/run_all_correctness.py --framework zipline --scenarios 01,03,0 ## Performance +Run the instrument-free runtime regression check with: + +```bash +uv run pytest tests/benchmark/test_hotpath_benchmarks.py::test_optimized_feed_runtime_vs_legacy_baseline --no-cov +``` + Benchmark on 250 assets x 20 years daily data (1.26M bars): | Metric | Value | diff --git a/docs/user-guide/results.md b/docs/user-guide/results.md index 128839cd..8d72358a 100644 --- a/docs/user-guide/results.md +++ b/docs/user-guide/results.md @@ -256,9 +256,10 @@ Returns a Polars DataFrame with columns: | `exit_reason_detail` | String | Detailed risk or liquidation cause, when available | | `status` | String | "closed", "partial", or "open" | -Partial reductions use `status="partial"`; lifecycle metrics exclude them to avoid counting one -position's holding period and excursions more than once. Open positions at the end of the backtest -use `status="open"` and mark-to-market values. +Partial reductions use `status="partial"`. Realized-P&L metrics include partial and fully closed +records, while holding-period and excursion metrics use only fully closed records to avoid counting +one position lifecycle more than once. Open positions at the end of the backtest use `status="open"` +and mark-to-market values. ## Equity DataFrame @@ -424,6 +425,10 @@ or ignored component. Unsupported manifest schema versions still fail because th is not defined. If an export selects `config` or `spec`, whether through the default component set or an explicit `include`, its absence or a serialization failure raises `ArtifactWriteError` instead of omitting the file. Component payloads are serialized before output files are created. +Non-finite metric floats use a tagged JSON object and are restored on load, so `metrics.json` +remains standards-compliant without losing `NaN` or infinity. The returned path mapping includes +`manifest`; passing its keys back through `include` treats that key as a no-op because the manifest +is always written. ## Integration with ml4t-diagnostic diff --git a/src/ml4t/backtest/accounting/gatekeeper.py b/src/ml4t/backtest/accounting/gatekeeper.py index 9d68ac1f..0c1e7933 100644 --- a/src/ml4t/backtest/accounting/gatekeeper.py +++ b/src/ml4t/backtest/accounting/gatekeeper.py @@ -187,6 +187,23 @@ def validate_order(self, order: Order, price: float) -> tuple[bool, str]: multiplier=multiplier, ) + def validate_order_with_code(self, order: Order, price: float) -> tuple[bool, str, str | None]: + """Validate an order and return a stable rejection code when invalid.""" + valid, reason = self.validate_order(order, price) + if valid: + return True, reason, None + + current_qty = self.account.get_position_quantity(order.asset) + quantity_delta = self._calculate_quantity_delta(order.side, order.quantity) + resulting_qty = current_qty + quantity_delta + if resulting_qty < 0 and not self.account.policy.allows_short_selling(): + code = "account_restriction" + elif getattr(self.account.policy, "allow_leverage", False): + code = "insufficient_buying_power" + else: + code = "insufficient_cash" + return False, reason, code + def _is_reversal(self, current_qty: float, order_qty_delta: float) -> bool: """Check if order reverses position (long → short or short → long). diff --git a/src/ml4t/backtest/analytics/trades.py b/src/ml4t/backtest/analytics/trades.py index cf625b2f..6b699fe3 100644 --- a/src/ml4t/backtest/analytics/trades.py +++ b/src/ml4t/backtest/analytics/trades.py @@ -15,12 +15,16 @@ class TradeAnalyzer: """Analyze a collection of trades for performance statistics.""" trades: Sequence["Trade"] + _lifecycle_trades: list["Trade"] = field(init=False, repr=False) def __post_init__(self): self._pnls = np.array([t.pnl for t in self.trades]) if self.trades else np.array([]) self._returns = ( np.array([t.pnl_percent for t in self.trades]) if self.trades else np.array([]) ) + self._lifecycle_trades = [ + trade for trade in self.trades if getattr(trade, "status", "closed") == "closed" + ] @property def num_trades(self) -> int: @@ -117,9 +121,9 @@ def payoff_ratio(self) -> float: @property def avg_bars_held(self) -> float: """Average number of bars positions were held.""" - if not self.trades: + if not self._lifecycle_trades: return 0.0 - bars = [t.bars_held for t in self.trades if hasattr(t, "bars_held")] + bars = [t.bars_held for t in self._lifecycle_trades if hasattr(t, "bars_held")] return float(np.mean(bars)) if bars else 0.0 @property @@ -190,17 +194,17 @@ def by_asset(self, asset: str) -> "TradeAnalyzer": @property def avg_mfe(self) -> float: """Average maximum favorable excursion across trades.""" - if not self.trades: + if not self._lifecycle_trades: return 0.0 - mfes = [t.mfe for t in self.trades] + mfes = [t.mfe for t in self._lifecycle_trades] return float(np.mean(mfes)) @property def avg_mae(self) -> float: """Average maximum adverse excursion across trades.""" - if not self.trades: + if not self._lifecycle_trades: return 0.0 - maes = [t.mae for t in self.trades] + maes = [t.mae for t in self._lifecycle_trades] return float(np.mean(maes)) @property @@ -210,10 +214,10 @@ def mfe_capture_ratio(self) -> float: Values close to 1.0 indicate exits near peak profit. Values close to 0.0 indicate exits gave back most gains. """ - if not self.trades: + if not self._lifecycle_trades: return 0.0 ratios = [] - for t in self.trades: + for t in self._lifecycle_trades: if t.mfe > 0: ratios.append(t.pnl_percent / t.mfe) return float(np.mean(ratios)) if ratios else 0.0 @@ -225,10 +229,10 @@ def mae_recovery_ratio(self) -> float: Calculated as (MAE - final_loss) / MAE for losing trades. Higher values indicate better recovery from drawdowns. """ - if not self.trades: + if not self._lifecycle_trades: return 0.0 ratios = [] - for t in self.trades: + for t in self._lifecycle_trades: if t.mae < 0 and t.pnl_percent < 0: # Both negative: MAE was -10%, final was -5% = recovered 50% recovery = (t.pnl_percent - t.mae) / abs(t.mae) diff --git a/src/ml4t/backtest/core/execution_engine.py b/src/ml4t/backtest/core/execution_engine.py index 86acf3f8..2f9b8fac 100644 --- a/src/ml4t/backtest/core/execution_engine.py +++ b/src/ml4t/backtest/core/execution_engine.py @@ -74,8 +74,10 @@ def _process_orders_exit_first( for order in eligible_orders: fill.apply_share_rounding(order) if order.quantity <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order.reject( + "Quantity rounds to zero (share_type=INTEGER)", + "quantity_rounds_to_zero", + ) continue if self._is_exit_order(order): exit_orders.append(order) @@ -181,8 +183,10 @@ def _process_orders_next_bar_queue_shadow(self, use_open: bool = False): fill.apply_share_rounding(order) if order.quantity <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order.reject( + "Quantity rounds to zero (share_type=INTEGER)", + "quantity_rounds_to_zero", + ) continue fill_price = fill.check_fill(order, price) @@ -190,15 +194,17 @@ def _process_orders_next_bar_queue_shadow(self, use_open: bool = False): continue validation_price = broker._current_prices.get(order.asset, fill_price) - valid, rejection_reason = self._validate_shadow_queue_order( + valid, rejection_reason, rejection_code = self._validate_shadow_queue_order( order=order, validation_price=validation_price, shadow_cash=shadow_cash, shadow_positions=shadow_positions, ) if not valid: - order.status = OrderStatus.REJECTED - order.rejection_reason = rejection_reason + order.reject( + rejection_reason, + rejection_code or "order_validation_failed", + ) continue accepted_orders.append((order, fill_price)) @@ -227,7 +233,7 @@ def _validate_shadow_queue_order( validation_price: float, shadow_cash: float, shadow_positions: dict[str, Position], - ) -> tuple[bool, str]: + ) -> tuple[bool, str, str | None]: broker = self.broker policy = broker.account.policy qty_delta = order.quantity if order.side is OrderSide.BUY else -order.quantity @@ -246,7 +252,7 @@ def _validate_shadow_queue_order( multiplier = broker.get_multiplier(order.asset) if abs(current_qty) <= 1e-12: - return policy.validate_new_position( + valid, reason = policy.validate_new_position( asset=order.asset, quantity=qty_delta, price=validation_price, @@ -254,8 +260,8 @@ def _validate_shadow_queue_order( cash=shadow_cash - commission, multiplier=multiplier, ) - if is_reversal: - return policy.handle_reversal( + elif is_reversal: + valid, reason = policy.handle_reversal( asset=order.asset, current_quantity=current_qty, order_quantity_delta=qty_delta, @@ -265,15 +271,24 @@ def _validate_shadow_queue_order( commission=commission, multiplier=multiplier, ) - return policy.validate_position_change( - asset=order.asset, - current_quantity=current_qty, - quantity_delta=qty_delta, - price=validation_price, - current_positions=shadow_positions, - cash=shadow_cash - commission, - multiplier=multiplier, - ) + else: + valid, reason = policy.validate_position_change( + asset=order.asset, + current_quantity=current_qty, + quantity_delta=qty_delta, + price=validation_price, + current_positions=shadow_positions, + cash=shadow_cash - commission, + multiplier=multiplier, + ) + + if valid: + return True, reason, None + if new_qty < 0 and not policy.allows_short_selling(): + return False, reason, "account_restriction" + if getattr(policy, "allow_leverage", False): + return False, reason, "insufficient_buying_power" + return False, reason, "insufficient_cash" def _commit_shadow_queue_fill( self, @@ -388,8 +403,10 @@ def _process_orders_sequential( fill.apply_share_rounding(order) if order.quantity <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order.reject( + "Quantity rounds to zero (share_type=INTEGER)", + "quantity_rounds_to_zero", + ) continue is_exit = self._is_exit_order(order) @@ -431,8 +448,10 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N fill.apply_share_rounding(order) if order.quantity <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order.reject( + "Quantity rounds to zero (share_type=INTEGER)", + "quantity_rounds_to_zero", + ) return is_exit = self._is_exit_order(order) @@ -441,8 +460,7 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N fill_price = fill.check_fill(order, price) if fill_price is not None: if use_simple_cash_check and not self._passes_simple_cash_check(order, fill_price): - order.status = OrderStatus.REJECTED - order.rejection_reason = "Insufficient cash (open cash check)" + order.reject("Insufficient cash (open cash check)", "insufficient_cash") return # Under locked-short-cash semantics, short covers/reversals can be @@ -456,15 +474,13 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N if broker.share_type.value == "integer": max_qty = float(int(max_qty)) if max_qty <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Insufficient cash to cover short" + order.reject("Insufficient cash to cover short", "insufficient_cash") return if max_qty < order.quantity: if broker.partial_fills_allowed: order.quantity = max_qty else: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Insufficient cash to cover short" + order.reject("Insufficient cash to cover short", "insufficient_cash") return fully_filled = fill.execute_fill(order, fill_price) @@ -479,8 +495,7 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N return if use_simple_cash_check and not self._passes_simple_cash_check(order, fill_price): - order.status = OrderStatus.REJECTED - order.rejection_reason = "Insufficient cash (open cash check)" + order.reject("Insufficient cash (open cash check)", "insufficient_cash") return # Under locked-short-cash semantics, reversal entries can be @@ -495,21 +510,22 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N if broker.share_type.value == "integer": max_qty = float(int(max_qty)) if max_qty <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Insufficient cash for reversal" + order.reject("Insufficient cash for reversal", "insufficient_cash") return if max_qty < order.quantity: if broker.partial_fills_allowed: order.quantity = max_qty else: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Insufficient cash for reversal" + order.reject("Insufficient cash for reversal", "insufficient_cash") return + rejection_code: str | None = None if skip_cash or use_simple_cash_check: valid, rejection_reason = True, "" else: - valid, rejection_reason = broker.gatekeeper.validate_order(order, fill_price) + valid, rejection_reason, rejection_code = ( + broker.gatekeeper.validate_order_with_code(order, fill_price) + ) insufficient_cash = "insufficient" in rejection_reason.lower() if valid: @@ -535,11 +551,15 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N filled_orders.append(order) broker._partial_orders.pop(order.order_id, None) else: - order.status = OrderStatus.REJECTED - order.rejection_reason = rejection_reason + order.reject( + rejection_reason, + rejection_code or "order_validation_failed", + ) else: - order.status = OrderStatus.REJECTED - order.rejection_reason = rejection_reason + order.reject( + rejection_reason, + rejection_code or "order_validation_failed", + ) def _use_simple_next_bar_cash_check(self, order, use_open: bool) -> bool: broker = self.broker diff --git a/src/ml4t/backtest/core/order_book.py b/src/ml4t/backtest/core/order_book.py index e0ecfe68..d945e3d9 100644 --- a/src/ml4t/backtest/core/order_book.py +++ b/src/ml4t/backtest/core/order_book.py @@ -82,17 +82,20 @@ def submit_order( if self._should_apply_submission_precheck(order) and not self._passes_submission_precheck( order ): - order.status = OrderStatus.REJECTED if not order.rejection_reason: order.rejection_reason = "Insufficient cash (submission precheck)" + order.reject(order.rejection_reason, order._rejection_code or "insufficient_cash") return order if self._should_apply_buying_power_reservation( order ) and not self._passes_buying_power_check(order): - order.status = OrderStatus.REJECTED if not order.rejection_reason: order.rejection_reason = "Insufficient buying power" + order.reject( + order.rejection_reason, + order._rejection_code or "insufficient_buying_power", + ) return order broker.pending_orders.append(order) @@ -134,21 +137,21 @@ def _fill_immediately(self, order: Order) -> Order: # Apply share rounding fill.apply_share_rounding(order) if order.quantity <= 0: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order.reject( + "Quantity rounds to zero (share_type=INTEGER)", + "quantity_rounds_to_zero", + ) return order # Get fill price (close price for same-bar) price = fill.get_fill_price_for_order(order, use_open=False) if price is None: - order.status = OrderStatus.REJECTED - order.rejection_reason = "No price available" + order.reject("No price available", "price_unavailable") return order fill_price = fill.check_fill(order, price) if fill_price is None: - order.status = OrderStatus.REJECTED - order.rejection_reason = "Fill check failed" + order.reject("Fill check failed", "fill_check_failed") return order # Determine if this is an exit (reduces existing position) @@ -164,27 +167,35 @@ def _fill_immediately(self, order: Order) -> Order: else: # Entries: validate against real cash via gatekeeper if not broker.skip_cash_validation: - valid, rejection_reason = broker.gatekeeper.validate_order(order, fill_price) + valid, rejection_reason, rejection_code = ( + broker.gatekeeper.validate_order_with_code(order, fill_price) + ) if not valid: allow_rebalance_partial = ( order.rebalance_id is not None and broker.share_type.value == "integer" ) if broker.partial_fills_allowed and "insufficient" in rejection_reason.lower(): if not fill.try_partial_fill(order, fill_price): - order.status = OrderStatus.REJECTED - order.rejection_reason = rejection_reason + order.reject( + rejection_reason, + rejection_code or "order_validation_failed", + ) return order broker._partial_orders.pop(order.order_id, None) return order if allow_rebalance_partial and "insufficient" in rejection_reason.lower(): if not fill.try_partial_fill(order, fill_price): - order.status = OrderStatus.REJECTED - order.rejection_reason = rejection_reason + order.reject( + rejection_reason, + rejection_code or "order_validation_failed", + ) return order broker._partial_orders.pop(order.order_id, None) return order - order.status = OrderStatus.REJECTED - order.rejection_reason = rejection_reason + order.reject( + rejection_reason, + rejection_code or "order_validation_failed", + ) return order fully_filled = fill.execute_fill(order, fill_price) @@ -314,6 +325,7 @@ def _passes_submission_precheck(self, order: Order) -> bool: order.quantity = float(int(order.quantity)) if order.quantity <= 0: order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order._rejection_code = "quantity_rounds_to_zero" return False signal_price = getattr(order, "_signal_price", None) @@ -399,6 +411,7 @@ def _passes_buying_power_check(self, order: Order) -> bool: order.quantity = float(int(order.quantity)) if order.quantity <= 0: order.rejection_reason = "Quantity rounds to zero (share_type=INTEGER)" + order._rejection_code = "quantity_rounds_to_zero" return False signal_price = getattr(order, "_signal_price", None) @@ -515,6 +528,11 @@ def _passes_margin_submission_precheck(self, order: Order, signal_price: float) if not valid: order.rejection_reason = reason or "Insufficient buying power (submission precheck)" + resulting_qty = old_qty + size + if resulting_qty < 0 and not broker.account.policy.allows_short_selling(): + order._rejection_code = "account_restriction" + else: + order._rejection_code = "insufficient_buying_power" return False multiplier = broker.get_multiplier(order.asset) diff --git a/src/ml4t/backtest/engine.py b/src/ml4t/backtest/engine.py index db7a13e1..361ecec8 100644 --- a/src/ml4t/backtest/engine.py +++ b/src/ml4t/backtest/engine.py @@ -433,9 +433,10 @@ def _generate_results(self) -> BacktestResult: ) all_trades.append(open_trade) - # Build TradeAnalyzer (only on closed trades for accurate stats) - closed_trades = [t for t in all_trades if t.status == "closed"] - trade_analyzer = TradeAnalyzer(closed_trades) + # Realized-P&L metrics include partial reductions. TradeAnalyzer limits + # lifecycle metrics such as holding period and excursions to full closes. + realized_trades = [t for t in all_trades if t.status in {"closed", "partial"}] + trade_analyzer = TradeAnalyzer(realized_trades) activity_metrics = self._build_activity_metrics() # Build metrics dictionary (backward compatible) diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index 52f6ceb7..4213fb82 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -182,6 +182,7 @@ def execute(self, order: Order, base_price: float) -> bool: close_commission = None open_commission = None position = broker.positions.get(order.asset) + is_exit_fill = position is not None and position.quantity * signed_qty < 0 if position is not None: new_qty = position.quantity + signed_qty if _is_position_flip(position.quantity, new_qty): @@ -228,18 +229,23 @@ def execute(self, order: Order, base_price: float) -> bool: bid_size=quote_context["bid_size"], ask_size=quote_context["ask_size"], available_size=quote_context["available_size"], - exit_reason=order._exit_reason.value if order._exit_reason is not None else "", + exit_reason=_get_exit_reason(order) if is_exit_fill else "", exit_reason_detail=order._risk_exit_reason, ) broker.fills.append(fill) # Determine if partial fill is_partial = order.order_id in broker._partial_orders - order.filled_quantity += fill_quantity + previous_filled_quantity = order.filled_quantity + cumulative_filled_quantity = previous_filled_quantity + fill_quantity + previous_fill_notional = (order.filled_price or 0.0) * previous_filled_quantity + order.filled_price = ( + previous_fill_notional + fill_price * fill_quantity + ) / cumulative_filled_quantity + order.filled_quantity = cumulative_filled_quantity if not is_partial: order.status = OrderStatus.FILLED order.filled_at = current_time - order.filled_price = fill_price # Build fill context ctx = FillContext( diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 2cec13e3..ec32df24 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -22,6 +22,7 @@ from __future__ import annotations import json +import math from dataclasses import dataclass, field from datetime import datetime from pathlib import Path @@ -46,6 +47,7 @@ _ARTIFACT_SCHEMA_VERSION = 1 _MANIFEST_FILE = "manifest.json" _INCOMPLETE_MARKER = ".artifact-incomplete" +_NONFINITE_FLOAT_KEY = "__ml4t_nonfinite_float__" _COMPONENT_FILES = { "trades": "trades.parquet", "fills": "fills.parquet", @@ -65,8 +67,18 @@ def _serialize_metric_value(value: Any, *, path: str) -> Any: """Convert a metric value to JSON-safe built-in containers and scalars.""" - if isinstance(value, int | float | str | bool | type(None)): + if isinstance(value, bool | str | type(None) | int): return value + if isinstance(value, float): + if math.isfinite(value): + return value + if math.isnan(value): + label = "nan" + elif value > 0: + label = "positive_infinity" + else: + label = "negative_infinity" + return {_NONFINITE_FLOAT_KEY: label} if isinstance(value, datetime): return value.isoformat() if isinstance(value, list | tuple): @@ -90,6 +102,25 @@ def _serialize_metric_value(value: Any, *, path: str) -> Any: raise ArtifactWriteError(f"{path} has unsupported value type {type(value).__name__}") +def _deserialize_metric_value(value: Any) -> Any: + """Restore tagged non-finite floats from a portable JSON payload.""" + if isinstance(value, list): + return [_deserialize_metric_value(item) for item in value] + if isinstance(value, dict): + if set(value) == {_NONFINITE_FLOAT_KEY}: + labels = { + "nan": float("nan"), + "positive_infinity": float("inf"), + "negative_infinity": float("-inf"), + } + label = value[_NONFINITE_FLOAT_KEY] + if label not in labels: + raise ValueError(f"Unknown non-finite metric label: {label!r}") + return labels[label] + return {key: _deserialize_metric_value(item) for key, item in value.items()} + return value + + @dataclass(frozen=True) class ArtifactDiagnostic: """Structured description of one omission or recovery action.""" @@ -669,9 +700,10 @@ def to_parquet( """ explicitly_selected = include is not None requested = list(include) if include is not None else list(_COMPONENT_FILES) - unknown = sorted(set(requested) - _COMPONENT_FILES.keys()) + unknown = sorted(set(requested) - _COMPONENT_FILES.keys() - {"manifest"}) if unknown: raise ArtifactWriteError(f"Unknown artifact components requested: {unknown}") + requested = [name for name in requested if name != "manifest"] unavailable: dict[str, str] = {} if self.predictions is None: @@ -694,7 +726,7 @@ def to_parquet( key: _serialize_metric_value(value, path=f"metrics[{key!r}]") for key, value in self.metrics.items() } - metrics_payload = json.dumps(serializable_metrics, indent=2) + metrics_payload = json.dumps(serializable_metrics, indent=2, allow_nan=False) except ArtifactWriteError: raise except Exception as exc: @@ -727,6 +759,8 @@ def to_parquet( path.mkdir(parents=True, exist_ok=True) marker_path = path / _INCOMPLETE_MARKER marker_path.write_text("Result artifact write did not complete.\n") + manifest_path = path / _MANIFEST_FILE + manifest_path.unlink(missing_ok=True) written: dict[str, Path] = {} @@ -800,10 +834,9 @@ def to_parquet( name: reason for name, reason in unavailable.items() if name in requested }, } - manifest_path = path / _MANIFEST_FILE try: with open(manifest_path, "w") as file: - json.dump(manifest, file, indent=2) + json.dump(manifest, file, indent=2, allow_nan=False) except Exception as exc: raise ArtifactWriteError(f"Failed to write artifact manifest: {exc}") from exc written["manifest"] = manifest_path @@ -905,6 +938,14 @@ def discover_legacy_components() -> dict[str, str]: ) components = discover_legacy_components() + if manifest is not None: + schema_version = manifest.get("schema_version") + if schema_version != _ARTIFACT_SCHEMA_VERSION: + raise UnsupportedArtifactVersionError( + f"Unsupported result artifact schema version {schema_version!r}; " + f"supported version is {_ARTIFACT_SCHEMA_VERSION}" + ) + if manifest is not None: artifact_type = manifest.get("artifact_type") if artifact_type != _ARTIFACT_TYPE: @@ -916,12 +957,6 @@ def discover_legacy_components() -> dict[str, str]: manifest = None if manifest is not None: - schema_version = manifest.get("schema_version") - if schema_version != _ARTIFACT_SCHEMA_VERSION: - raise UnsupportedArtifactVersionError( - f"Unsupported result artifact schema version {schema_version!r}; " - f"supported version is {_ARTIFACT_SCHEMA_VERSION}" - ) component_data = manifest.get("components") if not isinstance(component_data, dict) or not all( isinstance(name, str) and isinstance(filename, str) @@ -1144,7 +1179,7 @@ def read_metrics(component_path: Path) -> dict[str, Any]: data = json.load(file) if not isinstance(data, dict): raise TypeError("metrics root must be an object") - return data + return _deserialize_metric_value(data) def read_config(component_path: Path): import yaml diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index 9349bfbb..dcaf3e48 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -191,6 +191,8 @@ def rejection_code(self) -> str | None: return "price_unavailable" if "fill check" in reason: return "fill_check_failed" + if "not allowed" in reason: + return "account_restriction" if "buying power" in reason or "margin" in reason: return "insufficient_buying_power" if "cash" in reason or "insufficient" in reason: @@ -199,6 +201,12 @@ def rejection_code(self) -> str | None: return "account_restriction" return "order_validation_failed" + def reject(self, reason: str, code: str) -> None: + """Move the order to a rejected state with a stable reason code.""" + self.status = OrderStatus.REJECTED + self.rejection_reason = reason + self._rejection_code = code + @dataclass class Position: diff --git a/tests/benchmark/test_hotpath_benchmarks.py b/tests/benchmark/test_hotpath_benchmarks.py index 4d9630bd..c8374979 100644 --- a/tests/benchmark/test_hotpath_benchmarks.py +++ b/tests/benchmark/test_hotpath_benchmarks.py @@ -2,11 +2,12 @@ These benchmarks compare the current DataFeed implementation against a reference legacy implementation (captured from pre-optimization behavior). +Run the runtime regression check with ``--no-cov`` because instrumentation +materially changes the comparison. CI invokes its exact pytest node ID. """ from __future__ import annotations -import os from datetime import datetime, timedelta from statistics import median from time import perf_counter @@ -188,8 +189,6 @@ def test_optimized_feed_matches_legacy_output(): @pytest.mark.benchmark def test_optimized_feed_runtime_vs_legacy_baseline(): if _coverage_session_started(): - if os.environ.get("ML4T_REQUIRE_RUNTIME_BENCHMARK") == "1": - pytest.fail("CI requires the runtime benchmark to run without coverage") pytest.skip("Runtime benchmark requires coverage instrumentation to be disabled") prices, signals = _build_benchmark_data(n_bars=3000, n_assets=20) diff --git a/tests/test_broker.py b/tests/test_broker.py index 192410cb..deaa150f 100644 --- a/tests/test_broker.py +++ b/tests/test_broker.py @@ -1540,6 +1540,20 @@ def test_partial_rule_exit_preserves_detailed_reason(self): assert broker.trades[-1].exit_reason == "signal" assert broker.trades[-1].exit_reason_detail == "scale_out_5%_25%" + def test_signal_exit_reason_is_present_on_fill_and_trade(self): + broker = Broker(100000.0, NoCommission(), NoSlippage()) + mark_prices(broker, {"AAPL": 100.0}) + broker.submit_order("AAPL", 10.0, OrderSide.BUY) + broker._process_orders() + + mark_prices(broker, {"AAPL": 105.0}) + broker.submit_order("AAPL", 10.0, OrderSide.SELL) + broker._process_orders() + + assert broker.fills[0].exit_reason == "" + assert broker.fills[-1].exit_reason == "signal" + assert broker.trades[-1].exit_reason == "signal" + def test_evaluate_position_rules_exit_full_deferred(self): """Test EXIT_FULL action with defer_fill=True (NEXT_BAR_OPEN mode).""" from ml4t.backtest.risk.position.static import StopLoss diff --git a/tests/test_partial_close_accounting.py b/tests/test_partial_close_accounting.py index 517785d0..a740adcc 100644 --- a/tests/test_partial_close_accounting.py +++ b/tests/test_partial_close_accounting.py @@ -153,4 +153,8 @@ def test_partial_exit_scale_up_and_close_reconcile_end_to_end() -> None: assert sum(trade.fees for trade in result.trades) == pytest.approx( sum(fill.commission for fill in result.fills) ) - assert result.metrics["num_trades"] == 1 + assert result.metrics["num_trades"] == 2 + assert result.metrics["winning_trades"] == 2 + assert result.trade_analyzer is not None + assert result.trade_analyzer.avg_bars_held == result.trades[-1].bars_held + assert result.trade_analyzer.avg_mfe == result.trades[-1].mfe diff --git a/tests/test_rejected_order_results.py b/tests/test_rejected_order_results.py index 6585d2ad..640b1798 100644 --- a/tests/test_rejected_order_results.py +++ b/tests/test_rejected_order_results.py @@ -27,6 +27,22 @@ def on_data(self, timestamp, data, context, broker) -> None: pass +class _ShortOrder(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + if not broker.orders: + broker.submit_order("AAPL", 1.0, OrderSide.SELL) + + +class _CaptureOrder(Strategy): + def __init__(self, quantity: float) -> None: + self.quantity = quantity + self.order: Order | None = None + + def on_data(self, timestamp, data, context, broker) -> None: + if self.order is None: + self.order = broker.submit_order("AAPL", self.quantity) + + def _prices() -> pl.DataFrame: return pl.DataFrame( { @@ -94,6 +110,16 @@ def test_no_orders_and_all_orders_rejected_are_distinguishable() -> None: assert all_rejected.metrics["num_rejected_orders"] == 1 +def test_cash_account_short_rejection_has_structured_restriction_code() -> None: + result = _run(_ShortOrder()) + + assert len(result.rejected_orders) == 1 + rejected = result.rejected_orders[0] + assert rejected.rejection_reason == "Short selling not allowed in cash account" + assert rejected._rejection_code == "account_restriction" + assert rejected.rejection_code == "account_restriction" + + def test_rejected_orders_round_trip_through_result_artifact(tmp_path) -> None: result = _run(_UnaffordableOrder()) @@ -153,12 +179,50 @@ def test_partially_filled_then_rejected_order_is_reconcilable() -> None: assert record["remaining_quantity"] == 10.0 +def test_completed_multi_bar_order_accumulates_quantity_and_average_price() -> None: + strategy = _CaptureOrder(12.0) + prices = pl.DataFrame( + { + "timestamp": [datetime(2024, 1, day) for day in (2, 3, 4)], + "asset": ["AAPL"] * 3, + "open": [10.0, 20.0, 40.0], + "high": [10.0, 20.0, 40.0], + "low": [10.0, 20.0, 40.0], + "close": [10.0, 20.0, 40.0], + "volume": [5.0, 5.0, 5.0], + } + ) + + result = run_backtest( + prices=prices, + strategy=strategy, + config=BacktestConfig( + initial_cash=10_000.0, + execution_mode=ExecutionMode.SAME_BAR, + partial_fills_allowed=True, + ), + execution_limits=VolumeParticipationLimit(max_participation=1.0), + ) + + assert strategy.order is not None + assert strategy.order.status is OrderStatus.FILLED + assert strategy.order.filled_quantity == sum(fill.quantity for fill in result.fills) == 12.0 + expected_average = sum(fill.quantity * fill.price for fill in result.fills) / 12.0 + assert strategy.order.filled_price == pytest.approx(expected_average) + + @pytest.mark.parametrize( ("reason", "expected"), [ ("Insufficient cash to cover short", "insufficient_cash"), ("Insufficient buying power", "insufficient_buying_power"), - ("Short selling not allowed", "account_restriction"), + ("Position reversal not allowed in cash account", "account_restriction"), + ("Short selling not allowed in cash account", "account_restriction"), + ( + "Position reversal not allowed in cash account (current: 10, delta: -20)", + "account_restriction", + ), + ("Short positions not allowed in cash account", "account_restriction"), ("No price available", "price_unavailable"), ], ) diff --git a/tests/test_result.py b/tests/test_result.py index 8d300956..852e85ba 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import math import tempfile from datetime import datetime, timedelta from pathlib import Path @@ -873,6 +874,46 @@ def test_from_parquet_rejects_unsupported_schema(self, backtest_result: Backtest with pytest.raises(UnsupportedArtifactVersionError, match="999"): BacktestResult.from_parquet(path) + manifest["artifact_type"] = "foreign-result" + manifest_path.write_text(json.dumps(manifest)) + with pytest.raises(UnsupportedArtifactVersionError, match="999"): + BacktestResult.from_parquet(path, recovery=True) + + def test_foreign_artifact_type_fails_strict_and_recovers_current_schema( + self, backtest_result: BacktestResult + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + manifest_path = path / "manifest.json" + manifest = json.loads(manifest_path.read_text()) + manifest["artifact_type"] = "foreign-result" + manifest_path.write_text(json.dumps(manifest)) + + with pytest.raises(ArtifactManifestError, match="foreign-result"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.artifact_diagnostics[0].code == "manifest_invalid" + + def test_noncanonical_manifest_component_fails_strict_and_recovers( + self, backtest_result: BacktestResult + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + manifest_path = path / "manifest.json" + manifest = json.loads(manifest_path.read_text()) + manifest["components"]["trades"] = "other.parquet" + manifest_path.write_text(json.dumps(manifest)) + + with pytest.raises(ArtifactManifestError, match="noncanonical"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.artifact_diagnostics[0].code == "manifest_invalid" + assert len(recovered.trades) == len(backtest_result.trades) + def test_from_parquet_rejects_malformed_manifest(self, backtest_result: BacktestResult): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" @@ -946,6 +987,61 @@ def test_metrics_json_serialization(self, backtest_result: BacktestResult): assert metrics["monthly_returns"] == [0.01, -0.02] assert metrics["segments"] == {"train": [1, 2], "test": [3]} + def test_nonfinite_metrics_use_portable_json_and_round_trip( + self, backtest_result: BacktestResult + ): + backtest_result.metrics.update( + { + "profit_factor": float("inf"), + "negative": float("-inf"), + "nested": [float("nan")], + } + ) + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + + raw_metrics = (path / "metrics.json").read_text() + assert "Infinity" not in raw_metrics + assert "NaN" not in raw_metrics + json.loads(raw_metrics, parse_constant=lambda value: pytest.fail(value)) + + loaded = BacktestResult.from_parquet(path) + assert math.isinf(loaded.metrics["profit_factor"]) + assert loaded.metrics["profit_factor"] > 0 + assert math.isinf(loaded.metrics["negative"]) + assert loaded.metrics["negative"] < 0 + assert math.isnan(loaded.metrics["nested"][0]) + + def test_written_keys_can_be_reused_as_include(self, backtest_result: BacktestResult): + with tempfile.TemporaryDirectory() as tmpdir: + first = Path(tmpdir) / "first" + second = Path(tmpdir) / "second" + + written = backtest_result.to_parquet(first) + replicated = backtest_result.to_parquet(second, include=list(written)) + + assert set(replicated) == set(written) + + def test_failed_reexport_removes_stale_manifest( + self, backtest_result: BacktestResult, monkeypatch: pytest.MonkeyPatch + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + + def fail_write(*args, **kwargs): + raise OSError("simulated component write failure") + + monkeypatch.setattr(pl.DataFrame, "write_parquet", fail_write) + with pytest.raises(OSError, match="simulated"): + backtest_result.to_parquet(path) + + assert not (path / "manifest.json").exists() + assert (path / ".artifact-incomplete").exists() + with pytest.raises(ArtifactIncompleteError, match="did not complete"): + BacktestResult.from_parquet(path) + def test_metrics_json_rejects_unserializable_values(self): result = BacktestResult( trades=[], From 2312d5df9cd595fef1bde5bb95af0ae88c751acd Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:57:58 -0400 Subject: [PATCH 11/15] fix(results): complete artifact validation review --- docs/user-guide/results.md | 7 ++-- src/ml4t/backtest/result.py | 68 ++++++++++++++++++++----------------- tests/test_result.py | 31 +++++++++++++++++ 3 files changed, 71 insertions(+), 35 deletions(-) diff --git a/docs/user-guide/results.md b/docs/user-guide/results.md index 8d72358a..ed21432e 100644 --- a/docs/user-guide/results.md +++ b/docs/user-guide/results.md @@ -422,9 +422,10 @@ for diagnostic in result.artifact_diagnostics: Recovery reads supported components in a deterministic order and reports every missing, malformed, or ignored component. Unsupported manifest schema versions still fail because their interpretation -is not defined. If an export selects `config` or `spec`, whether through the default component set -or an explicit `include`, its absence or a serialization failure raises `ArtifactWriteError` -instead of omitting the file. Component payloads are serialized before output files are created. +is not defined. Missing `config` or `spec` data raises only when an explicit `include` requests it; +the default export records unavailable optional components in the manifest. A serialization failure +raises `ArtifactWriteError` whether the component came from the default set or an explicit +`include`. Component payloads are serialized before output files are created. Non-finite metric floats use a tagged JSON object and are restored on load, so `metrics.json` remains standards-compliant without losing `NaN` or infinity. The returned path mapping includes `manifest`; passing its keys back through `include` treats that key as a no-op because the manifest diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index ec32df24..d4b3a5ec 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -99,6 +99,12 @@ def _serialize_metric_value(value: Any, *, path: str) -> Any: return _serialize_metric_value(value.item(), path=path) except (ImportError, AttributeError): pass + to_list = getattr(value, "to_list", None) + if callable(to_list): + return _serialize_metric_value(to_list(), path=path) + tolist = getattr(value, "tolist", None) + if callable(tolist): + return _serialize_metric_value(tolist(), path=path) raise ArtifactWriteError(f"{path} has unsupported value type {type(value).__name__}") @@ -719,41 +725,47 @@ def to_parquet( selected = [name for name in requested if name not in unavailable] - metrics_payload: str | None = None + text_payloads: dict[str, str] = {} if "metrics" in selected: try: serializable_metrics = { key: _serialize_metric_value(value, path=f"metrics[{key!r}]") for key, value in self.metrics.items() } - metrics_payload = json.dumps(serializable_metrics, indent=2, allow_nan=False) + text_payloads["metrics"] = json.dumps( + serializable_metrics, + indent=2, + allow_nan=False, + ) except ArtifactWriteError: raise except Exception as exc: raise ArtifactWriteError(f"Failed to serialize metrics: {exc}") from exc - config_payload: str | None = None - spec_payload: str | None = None if "config" in selected or "spec" in selected: try: import yaml - - if "config" in selected: - config_payload = yaml.safe_dump( + except ImportError as exc: + raise ArtifactWriteError("PyYAML is required to serialize config or spec") from exc + if "config" in selected: + try: + text_payloads["config"] = yaml.safe_dump( self.config.to_dict(), default_flow_style=False, ) - if "spec" in selected: - spec_payload = yaml.safe_dump( + except Exception as exc: + raise ArtifactWriteError( + f"Failed to serialize config component: {exc}" + ) from exc + if "spec" in selected: + try: + text_payloads["spec"] = yaml.safe_dump( self.to_spec_dict(), default_flow_style=False, sort_keys=False, ) - except Exception as exc: - components = [name for name in ("config", "spec") if name in selected] - raise ArtifactWriteError( - f"Failed to serialize {' and '.join(components)} component: {exc}" - ) from exc + except Exception as exc: + raise ArtifactWriteError(f"Failed to serialize spec component: {exc}") from exc path = Path(path) path.mkdir(parents=True, exist_ok=True) @@ -804,23 +816,15 @@ def to_parquet( self.to_daily_pnl().write_parquet(daily_path, compression=compression) written["daily_pnl"] = daily_path - if "metrics" in selected: - metrics_path = path / "metrics.json" - assert metrics_payload is not None - metrics_path.write_text(metrics_payload) - written["metrics"] = metrics_path - - if "config" in selected: - config_path = path / "config.yaml" - assert config_payload is not None - config_path.write_text(config_payload) - written["config"] = config_path - - if "spec" in selected: - spec_path = path / "spec.yaml" - assert spec_payload is not None - spec_path.write_text(spec_payload) - written["spec"] = spec_path + for name in ("metrics", "config", "spec"): + if name not in selected: + continue + component_path = path / _COMPONENT_FILES[name] + try: + component_path.write_text(text_payloads[name]) + except Exception as exc: + raise ArtifactWriteError(f"Failed to write {name} component: {exc}") from exc + written[name] = component_path manifest = { "artifact_type": _ARTIFACT_TYPE, @@ -1215,7 +1219,7 @@ def read_spec_config(component_path: Path): portfolio_state = read_component("portfolio_state", read_portfolio_state, []) metrics = read_component("metrics", read_metrics, {}) predictions = read_component("predictions", pl.read_parquet, None) - read_component("daily_pnl", lambda value: pl.scan_parquet(value).collect_schema(), None) + read_component("daily_pnl", pl.read_parquet, None) config = read_component("config", read_config, None) spec_config = read_component("spec", read_spec_config, None) if config is None: diff --git a/tests/test_result.py b/tests/test_result.py index 852e85ba..2b4cfec4 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -9,6 +9,7 @@ from pathlib import Path from types import SimpleNamespace +import numpy as np import polars as pl import pytest from ml4t.specs.market_data import FeedSpec @@ -704,6 +705,23 @@ def to_dict(self): result.to_parquet(path, include=[component]) assert not path.exists() + def test_default_export_names_only_the_component_that_failed( + self, backtest_result: BacktestResult, monkeypatch: pytest.MonkeyPatch + ): + backtest_result.config = BacktestConfig() + + def fail_spec(): + raise ValueError("bad spec") + + monkeypatch.setattr(backtest_result, "to_spec_dict", fail_spec) + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + with pytest.raises(ArtifactWriteError, match="spec component") as exc_info: + backtest_result.to_parquet(path) + + assert "config and spec" not in str(exc_info.value) + assert not path.exists() + @pytest.mark.parametrize("component", ["config", "spec"]) def test_to_parquet_rejects_requested_config_without_config(self, component: str): result = BacktestResult(trades=[], equity_curve=[], fills=[], metrics={}) @@ -862,6 +880,15 @@ def test_from_parquet_rejects_corrupt_metrics(self, backtest_result: BacktestRes with pytest.raises(ArtifactReadError, match="metrics.json"): BacktestResult.from_parquet(path) + def test_from_parquet_rejects_corrupt_daily_pnl(self, backtest_result: BacktestResult): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / "daily_pnl.parquet").write_bytes(b"not parquet") + + with pytest.raises(ArtifactReadError, match="daily_pnl.parquet"): + BacktestResult.from_parquet(path) + def test_from_parquet_rejects_unsupported_schema(self, backtest_result: BacktestResult): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" @@ -975,6 +1002,8 @@ def test_metrics_json_serialization(self, backtest_result: BacktestResult): """Test metrics JSON contains only serializable values.""" backtest_result.metrics["monthly_returns"] = [0.01, -0.02] backtest_result.metrics["segments"] = {"train": (1, 2), "test": [3]} + backtest_result.metrics["array"] = np.array([1.0, 2.0]) + backtest_result.metrics["series"] = pl.Series([3.0, 4.0]) with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" backtest_result.to_parquet(path) @@ -986,6 +1015,8 @@ def test_metrics_json_serialization(self, backtest_result: BacktestResult): assert isinstance(metrics["final_value"], float) assert metrics["monthly_returns"] == [0.01, -0.02] assert metrics["segments"] == {"train": [1, 2], "test": [3]} + assert metrics["array"] == [1.0, 2.0] + assert metrics["series"] == [3.0, 4.0] def test_nonfinite_metrics_use_portable_json_and_round_trip( self, backtest_result: BacktestResult From 8b5985fc500f96c7fc7b6516a1032d4cdabc4f1e Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 14:59:06 -0400 Subject: [PATCH 12/15] fix(engine): define deterministic pre-risk state Closes #46. --- docs/user-guide/execution-semantics.md | 24 ++++++ src/ml4t/backtest/engine.py | 8 +- src/ml4t/backtest/strategy.py | 16 +++- tests/test_pre_risk_strategy.py | 107 ++++++++++++++++++++++++- 4 files changed, 152 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/execution-semantics.md b/docs/user-guide/execution-semantics.md index bd12e102..89e83111 100644 --- a/docs/user-guide/execution-semantics.md +++ b/docs/user-guide/execution-semantics.md @@ -25,6 +25,30 @@ config = BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR) # default `OrderType.MOC` is the exception. In `NEXT_BAR` mode, `MOC` orders submitted during `on_data()` still fill on the current bar, at the close, after strategy logic runs. +### Pre-Risk Callback State + +`Strategy.on_before_risk()` runs after the current bar has been registered and immediately before +position rules are evaluated. The state visible to the callback depends on execution mode: + +| Mode | Positions visible to `on_before_risk()` | Ordinary orders submitted there | +|------|------------------------------------------|----------------------------------| +| `NEXT_BAR` | Includes fills from prior bars at the current open | Pending until the next bar | +| `SAME_BAR` | State before regular pending-order processing | Processed during the current bar | + +In `SAME_BAR`, set `immediate_fill=True` when a position opened in `on_before_risk()` must receive +stop or trailing-rule evaluation on that same bar. In `NEXT_BAR`, a position guard is deterministic +because the prior opening order has filled before the callback: + +```python +def on_before_risk(self, timestamp, data, context, broker): + if broker.get_position("SPY") is None: + broker.submit_order("SPY", 10) +``` + +Use `broker.get_pending_orders("SPY")` when the strategy's sizing decision depends on unfilled +intent. Explicit pyramiding remains available by submitting an additional order without the +position guard. + ### SAME_BAR Orders fill at the **current bar's close** price, in the same bar they are submitted. diff --git a/src/ml4t/backtest/engine.py b/src/ml4t/backtest/engine.py index 361ecec8..906df49d 100644 --- a/src/ml4t/backtest/engine.py +++ b/src/ml4t/backtest/engine.py @@ -222,6 +222,11 @@ def run(self) -> BacktestResult: # This must happen BEFORE evaluate_position_rules() to clear deferred exits self.broker._process_pending_exits() + if self.execution_mode == ExecutionMode.NEXT_BAR: + # Fill orders submitted on prior bars before exposing state to the + # pre-risk callback. Orders submitted by the callback remain queued. + self.broker._process_orders(use_open=True) + # Optional strategy phase for opening orders that must receive risk # protection during the current bar. Existing strategies inherit a no-op. self.strategy.on_before_risk(timestamp, assets_data, context, self.broker) @@ -230,7 +235,8 @@ def run(self) -> BacktestResult: self.broker.evaluate_position_rules() if self.execution_mode == ExecutionMode.NEXT_BAR: - # Next-bar mode: process pending orders at open price + # Process same-cycle risk exits. Ordinary orders created by + # on_before_risk remain ineligible until the next bar. self.broker._process_orders(use_open=True) # Strategy generates new orders self.strategy.on_data(timestamp, assets_data, context, self.broker) diff --git a/src/ml4t/backtest/strategy.py b/src/ml4t/backtest/strategy.py index 3cefb765..b5e83dee 100644 --- a/src/ml4t/backtest/strategy.py +++ b/src/ml4t/backtest/strategy.py @@ -16,7 +16,21 @@ def on_before_risk( context: dict[str, Any], broker: Any, ) -> None: - """Called before position rules are evaluated for the current bar.""" + """Run strategy logic immediately before current-bar position risk. + + The broker has registered the current bar's prices before this callback. + In ``NEXT_BAR`` mode, orders from prior bars and deferred exits have also + filled at the current open, so position and pending-order queries expose + post-open state. Ordinary orders submitted here remain pending until the + next bar. In ``SAME_BAR`` mode, the callback runs before regular pending + orders are processed; a market order is visible to current-bar risk only + when ``immediate_fill=True``. + + A position guard therefore prevents duplicate next-bar entries. Strategies + can pyramid explicitly by submitting additional orders without that guard, + and can inspect ``broker.get_pending_orders(asset)`` when pending intent is + relevant to their sizing rule. + """ return None @abstractmethod diff --git a/tests/test_pre_risk_strategy.py b/tests/test_pre_risk_strategy.py index 0e014fbd..35383e70 100644 --- a/tests/test_pre_risk_strategy.py +++ b/tests/test_pre_risk_strategy.py @@ -1,6 +1,6 @@ """End-to-end tests for strategy work that must run before position risk.""" -from datetime import datetime +from datetime import datetime, timedelta import polars as pl @@ -41,6 +41,58 @@ def on_data(self, timestamp, data, context, broker) -> None: pass +class GuardedPreRiskEntry(Strategy): + """Enter only while no position exists and record callback-visible state.""" + + def __init__(self) -> None: + self.trace: list[tuple[str, int, float, int]] = [] + + def _record(self, phase: str, timestamp: datetime, broker: Broker) -> None: + position = broker.get_position("SPY") + self.trace.append( + ( + phase, + timestamp.day, + 0.0 if position is None else position.quantity, + len(broker.get_pending_orders("SPY")), + ) + ) + + def on_before_risk(self, timestamp, data, context, broker) -> None: + self._record("before_risk", timestamp, broker) + if broker.get_position("SPY") is None: + broker.submit_order("SPY", 10) + + def on_data(self, timestamp, data, context, broker) -> None: + self._record("on_data", timestamp, broker) + + +class ExplicitPreRiskPyramiding(Strategy): + """Submit an additional lot on every bar without a position guard.""" + + def on_before_risk(self, timestamp, data, context, broker) -> None: + broker.submit_order("SPY", 10) + + def on_data(self, timestamp, data, context, broker) -> None: + pass + + +def _daily_prices(days: int = 3) -> pl.DataFrame: + start = datetime(2026, 8, 3) + timestamps = [start + timedelta(days=offset) for offset in range(days)] + return pl.DataFrame( + { + "timestamp": timestamps, + "asset": ["SPY"] * days, + "open": [100.0] * days, + "high": [101.0] * days, + "low": [99.0] * days, + "close": [100.0] * days, + "volume": [1_000_000.0] * days, + } + ) + + def test_pre_risk_entry_can_trigger_stop_on_entry_bar(): """A position entered at the open receives stop protection on the same bar.""" prices = pl.DataFrame( @@ -137,3 +189,56 @@ def test_entry_bar_extreme_option_roundtrips_and_reaches_broker(): assert restored.trail_include_entry_bar_extremes is True assert broker.trail_include_entry_bar_extremes is True + + +def test_next_bar_pre_risk_guard_sees_filled_open_order() -> None: + strategy = GuardedPreRiskEntry() + result = Engine( + DataFeed(prices_df=_daily_prices()), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ).run() + + assert [(fill.timestamp.day, fill.quantity) for fill in result.fills] == [(4, 10.0)] + assert strategy.trace == [ + ("before_risk", 3, 0.0, 0), + ("on_data", 3, 0.0, 1), + ("before_risk", 4, 10.0, 0), + ("on_data", 4, 10.0, 0), + ("before_risk", 5, 10.0, 0), + ("on_data", 5, 10.0, 0), + ] + + +def test_same_bar_pre_risk_trace_is_stable() -> None: + strategy = GuardedPreRiskEntry() + result = Engine( + DataFeed(prices_df=_daily_prices(days=2)), + strategy, + BacktestConfig( + execution_mode=ExecutionMode.SAME_BAR, + immediate_fill=False, + ), + ).run() + + assert [(fill.timestamp.day, fill.quantity) for fill in result.fills] == [(3, 10.0)] + assert strategy.trace == [ + ("before_risk", 3, 0.0, 0), + ("on_data", 3, 10.0, 0), + ("before_risk", 4, 10.0, 0), + ("on_data", 4, 10.0, 0), + ] + + +def test_next_bar_pre_risk_allows_explicit_pyramiding() -> None: + engine = Engine( + DataFeed(prices_df=_daily_prices()), + ExplicitPreRiskPyramiding(), + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ) + + result = engine.run() + + assert sum(fill.quantity for fill in result.fills) == 20.0 + assert engine.broker.get_position("SPY").quantity == 20.0 + assert len(engine.broker.get_pending_orders("SPY")) == 1 From 204f2c58e9b73a3331a0eeab9948ea2b84589150 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 15:14:49 -0400 Subject: [PATCH 13/15] fix: preserve milestone execution and artifact contracts --- .github/workflows/ci.yml | 2 +- docs/user-guide/execution-semantics.md | 16 ++- docs/user-guide/results.md | 39 +++--- src/ml4t/backtest/accounting/gatekeeper.py | 16 ++- src/ml4t/backtest/analytics/trades.py | 28 ++-- src/ml4t/backtest/broker.py | 7 +- src/ml4t/backtest/core/execution_engine.py | 24 +++- src/ml4t/backtest/core/order_book.py | 8 +- src/ml4t/backtest/core/risk_engine.py | 5 +- src/ml4t/backtest/engine.py | 20 ++- src/ml4t/backtest/result.py | 142 +++++++++++++++------ src/ml4t/backtest/strategy.py | 23 ++-- src/ml4t/backtest/types.py | 2 + tests/benchmark/test_hotpath_benchmarks.py | 1 - tests/test_broker.py | 20 +++ tests/test_pre_risk_strategy.py | 130 ++++++++++++++++++- tests/test_rejected_order_results.py | 57 ++++++++- tests/test_result.py | 62 ++++++++- tests/test_trade_mfe_mae.py | 13 ++ 19 files changed, 494 insertions(+), 121 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 914ea575..c58ff3f1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,7 +77,7 @@ jobs: run: uv sync --dev - name: Run tests - run: uv run pytest tests/ -v --tb=short -x --no-cov + run: uv run pytest tests/ -v --tb=short -x --no-cov -m "not benchmark" - name: Run runtime regression benchmark run: >- diff --git a/docs/user-guide/execution-semantics.md b/docs/user-guide/execution-semantics.md index 89e83111..1ed6bc39 100644 --- a/docs/user-guide/execution-semantics.md +++ b/docs/user-guide/execution-semantics.md @@ -32,22 +32,24 @@ position rules are evaluated. The state visible to the callback depends on execu | Mode | Positions visible to `on_before_risk()` | Ordinary orders submitted there | |------|------------------------------------------|----------------------------------| -| `NEXT_BAR` | Includes fills from prior bars at the current open | Pending until the next bar | +| `NEXT_BAR` | Includes marketable flat-position entries previously submitted by this callback | Pending until the next bar | | `SAME_BAR` | State before regular pending-order processing | Processed during the current bar | In `SAME_BAR`, set `immediate_fill=True` when a position opened in `on_before_risk()` must receive -stop or trailing-rule evaluation on that same bar. In `NEXT_BAR`, a position guard is deterministic -because the prior opening order has filled before the callback: +stop or trailing-rule evaluation on that same bar. In `NEXT_BAR`, newly opened positions start risk +evaluation on the following bar, matching ordinary next-bar entry timing. A marketable prior entry +fills before the callback. Limit and stop orders can remain pending, so a guarded entry checks both +position and pending intent: ```python def on_before_risk(self, timestamp, data, context, broker): - if broker.get_position("SPY") is None: + if broker.get_position("SPY") is None and not broker.get_pending_orders("SPY"): broker.submit_order("SPY", 10) ``` -Use `broker.get_pending_orders("SPY")` when the strategy's sizing decision depends on unfilled -intent. Explicit pyramiding remains available by submitting an additional order without the -position guard. +Orders submitted by `on_data()` retain the configured within-bar fill ordering with risk exits. +Explicit pyramiding remains available by submitting an additional order without the flat-position +and pending-order guard. ### SAME_BAR diff --git a/docs/user-guide/results.md b/docs/user-guide/results.md index ed21432e..165874ba 100644 --- a/docs/user-guide/results.md +++ b/docs/user-guide/results.md @@ -87,22 +87,22 @@ print(f"Net PF: {m['profit_factor']:.2f}") | `sharpe` | Sharpe ratio | | `sortino` | Sortino ratio | | `calmar` | Calmar ratio | -| `num_trades` | Total completed trades | +| `num_trades` | Realized exit legs, including partial reductions and full closes | | `num_orders` | Total submitted orders | | `num_rejected_orders` | Orders that reached the rejected terminal state | | `num_fills` | Total execution events | | `num_rebalance_events` | Unique timestamps with at least one fill | | `unique_symbols_traded` | Number of symbols with at least one fill | -| `winning_trades` | Number of winning trades | -| `losing_trades` | Number of losing trades | -| `win_rate` | Win rate (0 to 1) | -| `profit_factor` | Net profit factor (winning P&L / losing P&L) | -| `expectancy` | Expected return per trade (decimal) | -| `avg_trade` | Average trade return (decimal) | -| `avg_win` | Average winning trade return (decimal) | -| `avg_loss` | Average losing trade return (decimal, negative) | -| `largest_win` | Best single trade return (decimal) | -| `largest_loss` | Worst single trade return (decimal, negative) | +| `winning_trades` | Winning realized exit legs | +| `losing_trades` | Losing realized exit legs | +| `win_rate` | Winning fraction across realized exit legs (0 to 1) | +| `profit_factor` | Winning P&L / losing P&L across realized exit legs | +| `expectancy` | Expected return per realized exit leg (decimal) | +| `avg_trade` | Average realized exit-leg return (decimal) | +| `avg_win` | Average winning realized exit-leg return (decimal) | +| `avg_loss` | Average losing realized exit-leg return (decimal, negative) | +| `largest_win` | Best realized exit-leg return (decimal) | +| `largest_loss` | Worst realized exit-leg return (decimal, negative) | | `payoff_ratio` | avg_win / \|avg_loss\| (size-normalized reward-to-risk) | | `total_commission` | Total commission paid | | `total_slippage` | Total slippage cost in dollars (entry + exit) | @@ -183,7 +183,9 @@ Quote-aware backtests therefore leave an explicit audit trail: ## Trade Analyzer -`result.trade_analyzer` provides aggregate statistics on closed trades: +`result.trade_analyzer` computes P&L statistics from realized exit legs, including partial +reductions. Holding-period and MAE/MFE statistics use fully closed position lifecycles. Those +lifecycle values are `NaN` when partial realizations exist but no position has fully closed. ```python ta = result.trade_analyzer @@ -405,11 +407,14 @@ from ml4t.backtest.result import BacktestResult result = BacktestResult.from_parquet("./results/my_backtest") ``` -`manifest.json` identifies artifact schema version 1 and every component written. Loading is +`manifest.json` identifies artifact schema version 2 and every component written. Loading is strict by default. An empty directory, missing manifest or required component, interrupted write, malformed file, or unsupported schema version raises a specific `ArtifactError` subclass before a result is returned. Selective exports are valid component exports, but they are not complete result -artifacts unless they contain all required components. +artifacts unless they contain all required components. Schema 1 was an unreleased development +format whose metrics used non-standard bare JSON constants for `NaN` and infinity; current readers +reject it rather than silently changing metric types. The stored `daily_pnl` component must decode +and equal the value recomputed from the stored equity curve. Manifest-free beta artifacts require explicit recovery: @@ -427,9 +432,9 @@ the default export records unavailable optional components in the manifest. A se raises `ArtifactWriteError` whether the component came from the default set or an explicit `include`. Component payloads are serialized before output files are created. Non-finite metric floats use a tagged JSON object and are restored on load, so `metrics.json` -remains standards-compliant without losing `NaN` or infinity. The returned path mapping includes -`manifest`; passing its keys back through `include` treats that key as a no-op because the manifest -is always written. +remains standards-compliant without losing `NaN` or infinity. NumPy arrays and Polars Series are +stored and loaded as JSON lists. The returned path mapping includes `manifest`; passing its keys +back through `include` treats that key as a no-op because the manifest is always written. ## Integration with ml4t-diagnostic diff --git a/src/ml4t/backtest/accounting/gatekeeper.py b/src/ml4t/backtest/accounting/gatekeeper.py index 0c1e7933..9e47c23e 100644 --- a/src/ml4t/backtest/accounting/gatekeeper.py +++ b/src/ml4t/backtest/accounting/gatekeeper.py @@ -196,13 +196,15 @@ def validate_order_with_code(self, order: Order, price: float) -> tuple[bool, st current_qty = self.account.get_position_quantity(order.asset) quantity_delta = self._calculate_quantity_delta(order.side, order.quantity) resulting_qty = current_qty + quantity_delta - if resulting_qty < 0 and not self.account.policy.allows_short_selling(): - code = "account_restriction" - elif getattr(self.account.policy, "allow_leverage", False): - code = "insufficient_buying_power" - else: - code = "insufficient_cash" - return False, reason, code + return False, reason, self.classify_rejection(resulting_qty) + + def classify_rejection(self, resulting_quantity: float) -> str: + """Classify a policy rejection independently of its display text.""" + if resulting_quantity < 0 and not self.account.policy.allows_short_selling(): + return "account_restriction" + if getattr(self.account.policy, "allow_leverage", False): + return "insufficient_buying_power" + return "insufficient_cash" def _is_reversal(self, current_qty: float, order_qty_delta: float) -> bool: """Check if order reverses position (long → short or short → long). diff --git a/src/ml4t/backtest/analytics/trades.py b/src/ml4t/backtest/analytics/trades.py index 6b699fe3..15a3f022 100644 --- a/src/ml4t/backtest/analytics/trades.py +++ b/src/ml4t/backtest/analytics/trades.py @@ -12,7 +12,13 @@ @dataclass class TradeAnalyzer: - """Analyze a collection of trades for performance statistics.""" + """Analyze realized exit legs and full-close position lifecycles. + + P&L statistics use every supplied realized exit leg, including partial + reductions. Holding-period and excursion statistics use only records whose + status is ``"closed"``. If realized legs exist but no position lifecycle has + closed, lifecycle statistics return NaN instead of an unmeasured zero. + """ trades: Sequence["Trade"] _lifecycle_trades: list["Trade"] = field(init=False, repr=False) @@ -120,9 +126,9 @@ def payoff_ratio(self) -> float: @property def avg_bars_held(self) -> float: - """Average number of bars positions were held.""" + """Average bars held across fully closed position lifecycles.""" if not self._lifecycle_trades: - return 0.0 + return float("nan") if self.trades else 0.0 bars = [t.bars_held for t in self._lifecycle_trades if hasattr(t, "bars_held")] return float(np.mean(bars)) if bars else 0.0 @@ -193,29 +199,29 @@ def by_asset(self, asset: str) -> "TradeAnalyzer": @property def avg_mfe(self) -> float: - """Average maximum favorable excursion across trades.""" + """Average maximum favorable excursion across fully closed lifecycles.""" if not self._lifecycle_trades: - return 0.0 + return float("nan") if self.trades else 0.0 mfes = [t.mfe for t in self._lifecycle_trades] return float(np.mean(mfes)) @property def avg_mae(self) -> float: - """Average maximum adverse excursion across trades.""" + """Average maximum adverse excursion across fully closed lifecycles.""" if not self._lifecycle_trades: - return 0.0 + return float("nan") if self.trades else 0.0 maes = [t.mae for t in self._lifecycle_trades] return float(np.mean(maes)) @property def mfe_capture_ratio(self) -> float: - """Average ratio of realized return to MFE. + """Average ratio of realized return to MFE for fully closed lifecycles. Values close to 1.0 indicate exits near peak profit. Values close to 0.0 indicate exits gave back most gains. """ if not self._lifecycle_trades: - return 0.0 + return float("nan") if self.trades else 0.0 ratios = [] for t in self._lifecycle_trades: if t.mfe > 0: @@ -224,13 +230,13 @@ def mfe_capture_ratio(self) -> float: @property def mae_recovery_ratio(self) -> float: - """Average ratio showing how much of MAE was recovered. + """Average MAE recovery ratio for fully closed position lifecycles. Calculated as (MAE - final_loss) / MAE for losing trades. Higher values indicate better recovery from drawdowns. """ if not self._lifecycle_trades: - return 0.0 + return float("nan") if self.trades else 0.0 ratios = [] for t in self._lifecycle_trades: if t.mae < 0 and t.pnl_percent < 0: diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 9fc7193a..6f80a8e9 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -219,6 +219,7 @@ def __init__( self._rebalance_counter = 0 self._orders_this_bar: list[Order] = [] # Orders placed this bar (for next-bar mode) self._orders_this_bar_ids: set[str] = set() + self._submitting_before_risk = False # Risk management self._position_rules: Any = None # Global position rules @@ -804,13 +805,13 @@ def update_position_context(self, asset: str, context: dict) -> None: if pos: pos.context.update(context) - def evaluate_position_rules(self) -> list[Order]: + def evaluate_position_rules(self, *, skip_assets: set[str] | None = None) -> list[Order]: """Evaluate position rules for all open positions. Called by Engine before processing orders. Returns list of exit orders. Handles defer_fill=True by storing pending exits for next bar. """ - return self._risk_engine.evaluate_position_rules() + return self._risk_engine.evaluate_position_rules(skip_assets=skip_assets) def submit_order( self, @@ -1770,6 +1771,7 @@ def _process_orders( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ): """Process pending orders against current prices. @@ -1788,4 +1790,5 @@ def _process_orders( use_open=use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) diff --git a/src/ml4t/backtest/core/execution_engine.py b/src/ml4t/backtest/core/execution_engine.py index 2f9b8fac..7fafa6af 100644 --- a/src/ml4t/backtest/core/execution_engine.py +++ b/src/ml4t/backtest/core/execution_engine.py @@ -21,11 +21,13 @@ def process_orders( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ): if self._should_use_next_bar_queue_shadow_validation( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ): self._process_orders_next_bar_queue_shadow(use_open) return @@ -36,18 +38,21 @@ def process_orders( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) elif ordering == "sequential": self._process_orders_sequential( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) else: self._process_orders_fifo( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) def _is_exit_order(self, order) -> bool: @@ -60,6 +65,7 @@ def _process_orders_exit_first( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ): broker = self.broker fill = broker._fill_engine @@ -69,6 +75,7 @@ def _process_orders_exit_first( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) for order in eligible_orders: @@ -127,12 +134,14 @@ def _should_use_next_bar_queue_shadow_validation( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ) -> bool: broker = self.broker if not ( use_open and broker.execution_mode is ExecutionMode.NEXT_BAR and broker.next_bar_queue_shadow_validation + and not only_pre_risk_flat_entries ): return False if order_types is not None or include_orders_this_bar: @@ -284,11 +293,7 @@ def _validate_shadow_queue_order( if valid: return True, reason, None - if new_qty < 0 and not policy.allows_short_selling(): - return False, reason, "account_restriction" - if getattr(policy, "allow_leverage", False): - return False, reason, "insufficient_buying_power" - return False, reason, "insufficient_cash" + return False, reason, broker.gatekeeper.classify_rejection(new_qty) def _commit_shadow_queue_fill( self, @@ -343,12 +348,14 @@ def _process_orders_fifo( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ): broker = self.broker eligible_orders = self._eligible_orders( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) filled_orders: list = [] @@ -366,6 +373,7 @@ def _process_orders_sequential( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ): """Process orders in submission order without exit/entry separation. @@ -386,6 +394,7 @@ def _process_orders_sequential( use_open, order_types=order_types, include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, ) filled_orders: list = [] @@ -576,6 +585,7 @@ def _eligible_orders( *, order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, ) -> list: broker = self.broker eligible_orders = [] @@ -585,6 +595,10 @@ def _eligible_orders( continue if order_types is not None and order.order_type not in order_types: continue + if only_pre_risk_flat_entries and not ( + order._submitted_before_risk and order._submitted_from_flat + ): + continue if ( broker.execution_mode is ExecutionMode.NEXT_BAR and order.order_id in orders_this_bar_ids diff --git a/src/ml4t/backtest/core/order_book.py b/src/ml4t/backtest/core/order_book.py index d945e3d9..8861853a 100644 --- a/src/ml4t/backtest/core/order_book.py +++ b/src/ml4t/backtest/core/order_book.py @@ -66,6 +66,8 @@ def submit_order( _risk_exit_reason=options.risk_exit_reason if options is not None else None, _exit_reason=options.exit_reason if options is not None else None, _risk_fill_price=options.risk_fill_price if options is not None else None, + _submitted_before_risk=broker._submitting_before_risk, + _submitted_from_flat=broker.get_position(asset) is None, ) order._signal_price = broker._current_prices.get(asset) @@ -528,11 +530,7 @@ def _passes_margin_submission_precheck(self, order: Order, signal_price: float) if not valid: order.rejection_reason = reason or "Insufficient buying power (submission precheck)" - resulting_qty = old_qty + size - if resulting_qty < 0 and not broker.account.policy.allows_short_selling(): - order._rejection_code = "account_restriction" - else: - order._rejection_code = "insufficient_buying_power" + order._rejection_code = broker.gatekeeper.classify_rejection(old_qty + size) return False multiplier = broker.get_multiplier(order.asset) diff --git a/src/ml4t/backtest/core/risk_engine.py b/src/ml4t/backtest/core/risk_engine.py index a4216b8f..a8d6bf77 100644 --- a/src/ml4t/backtest/core/risk_engine.py +++ b/src/ml4t/backtest/core/risk_engine.py @@ -13,11 +13,14 @@ class RiskEngine: def __init__(self, broker): self.broker = broker - def evaluate_position_rules(self): + def evaluate_position_rules(self, *, skip_assets: set[str] | None = None): broker = self.broker exit_orders = [] + skipped = skip_assets or set() for asset, pos in list(broker.positions.items()): + if asset in skipped: + continue rules = self._get_position_rules(asset) if rules is None: continue diff --git a/src/ml4t/backtest/engine.py b/src/ml4t/backtest/engine.py index 906df49d..a2bb1243 100644 --- a/src/ml4t/backtest/engine.py +++ b/src/ml4t/backtest/engine.py @@ -222,17 +222,27 @@ def run(self) -> BacktestResult: # This must happen BEFORE evaluate_position_rules() to clear deferred exits self.broker._process_pending_exits() + pre_risk_opened_assets: set[str] = set() if self.execution_mode == ExecutionMode.NEXT_BAR: - # Fill orders submitted on prior bars before exposing state to the - # pre-risk callback. Orders submitted by the callback remain queued. - self.broker._process_orders(use_open=True) + # Fill only flat-position entries submitted by this callback on a + # prior bar. Ordinary orders retain the configured exit-first batch. + positions_before = set(self.broker.positions) + self.broker._process_orders( + use_open=True, + only_pre_risk_flat_entries=True, + ) + pre_risk_opened_assets = set(self.broker.positions) - positions_before # Optional strategy phase for opening orders that must receive risk # protection during the current bar. Existing strategies inherit a no-op. - self.strategy.on_before_risk(timestamp, assets_data, context, self.broker) + self.broker._submitting_before_risk = True + try: + self.strategy.on_before_risk(timestamp, assets_data, context, self.broker) + finally: + self.broker._submitting_before_risk = False # Evaluate position rules (stops, trails, etc.) - generates exit orders - self.broker.evaluate_position_rules() + self.broker.evaluate_position_rules(skip_assets=pre_risk_opened_assets) if self.execution_mode == ExecutionMode.NEXT_BAR: # Process same-cycle risk exits. Ordinary orders created by diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index d4b3a5ec..08fbe286 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -44,7 +44,7 @@ _ARTIFACT_TYPE = "ml4t-backtest-result" -_ARTIFACT_SCHEMA_VERSION = 1 +_ARTIFACT_SCHEMA_VERSION = 2 _MANIFEST_FILE = "manifest.json" _INCOMPLETE_MARKER = ".artifact-incomplete" _NONFINITE_FLOAT_KEY = "__ml4t_nonfinite_float__" @@ -97,14 +97,12 @@ def _serialize_metric_value(value: Any, *, path: str) -> Any: if isinstance(value, np.generic): return _serialize_metric_value(value.item(), path=path) + if isinstance(value, np.ndarray): + return _serialize_metric_value(value.tolist(), path=path) except (ImportError, AttributeError): pass - to_list = getattr(value, "to_list", None) - if callable(to_list): - return _serialize_metric_value(to_list(), path=path) - tolist = getattr(value, "tolist", None) - if callable(tolist): - return _serialize_metric_value(tolist(), path=path) + if isinstance(value, pl.Series): + return _serialize_metric_value(value.to_list(), path=path) raise ArtifactWriteError(f"{path} has unsupported value type {type(value).__name__}") @@ -768,62 +766,114 @@ def to_parquet( raise ArtifactWriteError(f"Failed to serialize spec component: {exc}") from exc path = Path(path) - path.mkdir(parents=True, exist_ok=True) + try: + path.mkdir(parents=True, exist_ok=True) + except Exception as exc: + raise ArtifactWriteError(f"Failed to create artifact directory {path}: {exc}") from exc + + def write_component(name: str, writer) -> None: + try: + writer() + except Exception as exc: + raise ArtifactWriteError(f"Failed to write {name} component: {exc}") from exc + marker_path = path / _INCOMPLETE_MARKER - marker_path.write_text("Result artifact write did not complete.\n") + write_component( + "incomplete marker", + lambda: marker_path.write_text("Result artifact write did not complete.\n"), + ) manifest_path = path / _MANIFEST_FILE - manifest_path.unlink(missing_ok=True) + write_component("stale manifest removal", lambda: manifest_path.unlink(missing_ok=True)) written: dict[str, Path] = {} if "trades" in selected: trades_path = path / "trades.parquet" - self.to_trades_dataframe().write_parquet(trades_path, compression=compression) + write_component( + "trades", + lambda: self.to_trades_dataframe().write_parquet( + trades_path, + compression=compression, + ), + ) written["trades"] = trades_path if "fills" in selected: fills_path = path / "fills.parquet" - self.to_fills_dataframe().write_parquet(fills_path, compression=compression) + write_component( + "fills", + lambda: self.to_fills_dataframe().write_parquet( + fills_path, + compression=compression, + ), + ) written["fills"] = fills_path if "rejected_orders" in selected: rejected_orders_path = path / "rejected_orders.parquet" - self.to_rejected_orders_dataframe().write_parquet( - rejected_orders_path, - compression=compression, + write_component( + "rejected_orders", + lambda: self.to_rejected_orders_dataframe().write_parquet( + rejected_orders_path, + compression=compression, + ), ) written["rejected_orders"] = rejected_orders_path if "predictions" in selected: predictions_path = path / "predictions.parquet" - self.to_predictions_dataframe().write_parquet(predictions_path, compression=compression) + write_component( + "predictions", + lambda: self.to_predictions_dataframe().write_parquet( + predictions_path, + compression=compression, + ), + ) written["predictions"] = predictions_path if "equity" in selected: equity_path = path / "equity.parquet" - self.to_equity_dataframe().write_parquet(equity_path, compression=compression) + write_component( + "equity", + lambda: self.to_equity_dataframe().write_parquet( + equity_path, + compression=compression, + ), + ) written["equity"] = equity_path if "portfolio_state" in selected: portfolio_state_path = path / "portfolio_state.parquet" - self.to_portfolio_state_dataframe().write_parquet( - portfolio_state_path, compression=compression + write_component( + "portfolio_state", + lambda: self.to_portfolio_state_dataframe().write_parquet( + portfolio_state_path, + compression=compression, + ), ) written["portfolio_state"] = portfolio_state_path if "daily_pnl" in selected: daily_path = path / "daily_pnl.parquet" - self.to_daily_pnl().write_parquet(daily_path, compression=compression) + write_component( + "daily_pnl", + lambda: self.to_daily_pnl().write_parquet( + daily_path, + compression=compression, + ), + ) written["daily_pnl"] = daily_path for name in ("metrics", "config", "spec"): if name not in selected: continue component_path = path / _COMPONENT_FILES[name] - try: - component_path.write_text(text_payloads[name]) - except Exception as exc: - raise ArtifactWriteError(f"Failed to write {name} component: {exc}") from exc + write_component( + name, + lambda component_path=component_path, payload=text_payloads[name]: ( + component_path.write_text(payload) + ), + ) written[name] = component_path manifest = { @@ -838,13 +888,10 @@ def to_parquet( name: reason for name, reason in unavailable.items() if name in requested }, } - try: - with open(manifest_path, "w") as file: - json.dump(manifest, file, indent=2, allow_nan=False) - except Exception as exc: - raise ArtifactWriteError(f"Failed to write artifact manifest: {exc}") from exc + manifest_payload = json.dumps(manifest, indent=2, allow_nan=False) + write_component("manifest", lambda: manifest_path.write_text(manifest_payload)) written["manifest"] = manifest_path - marker_path.unlink() + write_component("incomplete marker removal", marker_path.unlink) return written @@ -942,14 +989,6 @@ def discover_legacy_components() -> dict[str, str]: ) components = discover_legacy_components() - if manifest is not None: - schema_version = manifest.get("schema_version") - if schema_version != _ARTIFACT_SCHEMA_VERSION: - raise UnsupportedArtifactVersionError( - f"Unsupported result artifact schema version {schema_version!r}; " - f"supported version is {_ARTIFACT_SCHEMA_VERSION}" - ) - if manifest is not None: artifact_type = manifest.get("artifact_type") if artifact_type != _ARTIFACT_TYPE: @@ -961,6 +1000,12 @@ def discover_legacy_components() -> dict[str, str]: manifest = None if manifest is not None: + schema_version = manifest.get("schema_version") + if schema_version != _ARTIFACT_SCHEMA_VERSION: + raise UnsupportedArtifactVersionError( + f"Unsupported result artifact schema version {schema_version!r}; " + f"supported version is {_ARTIFACT_SCHEMA_VERSION}" + ) component_data = manifest.get("components") if not isinstance(component_data, dict) or not all( isinstance(name, str) and isinstance(filename, str) @@ -1219,12 +1264,31 @@ def read_spec_config(component_path: Path): portfolio_state = read_component("portfolio_state", read_portfolio_state, []) metrics = read_component("metrics", read_metrics, {}) predictions = read_component("predictions", pl.read_parquet, None) - read_component("daily_pnl", pl.read_parquet, None) + daily_pnl = read_component("daily_pnl", pl.read_parquet, None) config = read_component("config", read_config, None) spec_config = read_component("spec", read_spec_config, None) if config is None: config = spec_config + if daily_pnl is not None: + expected_daily_pnl = cls( + trades=[], + equity_curve=equity_curve, + fills=[], + metrics={}, + ).to_daily_pnl() + if not daily_pnl.equals(expected_daily_pnl): + message = "daily_pnl.parquet is inconsistent with equity.parquet" + if not recovery: + raise ArtifactReadError(message) + diagnostics.append( + ArtifactDiagnostic( + code="component_inconsistent", + component="daily_pnl", + message=message, + ) + ) + return cls( trades=trades, equity_curve=equity_curve, diff --git a/src/ml4t/backtest/strategy.py b/src/ml4t/backtest/strategy.py index b5e83dee..8b95dc0d 100644 --- a/src/ml4t/backtest/strategy.py +++ b/src/ml4t/backtest/strategy.py @@ -19,17 +19,18 @@ def on_before_risk( """Run strategy logic immediately before current-bar position risk. The broker has registered the current bar's prices before this callback. - In ``NEXT_BAR`` mode, orders from prior bars and deferred exits have also - filled at the current open, so position and pending-order queries expose - post-open state. Ordinary orders submitted here remain pending until the - next bar. In ``SAME_BAR`` mode, the callback runs before regular pending - orders are processed; a market order is visible to current-bar risk only - when ``immediate_fill=True``. - - A position guard therefore prevents duplicate next-bar entries. Strategies - can pyramid explicitly by submitting additional orders without that guard, - and can inspect ``broker.get_pending_orders(asset)`` when pending intent is - relevant to their sizing rule. + In ``NEXT_BAR`` mode, a marketable flat-position entry submitted by this + callback on a prior bar fills at the current open before the callback runs. + Newly opened positions start risk evaluation on the following bar, preserving + next-bar timing. Untriggered limit or stop orders remain pending, so guarded + entries must check both ``broker.get_position(asset)`` and + ``broker.get_pending_orders(asset)``. Ordinary orders submitted here remain + pending until the next bar. In ``SAME_BAR`` mode, the callback runs before + regular pending orders are processed; a market order is visible to current-bar + risk only when ``immediate_fill=True``. + + Strategies can pyramid explicitly by submitting additional orders without + the flat-position and pending-order guard. """ return None diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index dcaf3e48..5c4c2006 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -172,6 +172,8 @@ class Order: _risk_exit_reason: str | None = None # Human-readable reason (legacy, for logging) _exit_reason: ExitReason | None = None # Typed exit reason (preferred) _risk_fill_price: float | None = None # Stop/target price for risk exits + _submitted_before_risk: bool = False + _submitted_from_flat: bool = False def __post_init__(self) -> None: if self.requested_quantity is None: diff --git a/tests/benchmark/test_hotpath_benchmarks.py b/tests/benchmark/test_hotpath_benchmarks.py index c8374979..3e770b04 100644 --- a/tests/benchmark/test_hotpath_benchmarks.py +++ b/tests/benchmark/test_hotpath_benchmarks.py @@ -170,7 +170,6 @@ def _coverage_session_started() -> bool: return Coverage.current() is not None -@pytest.mark.benchmark def test_optimized_feed_matches_legacy_output(): prices, signals = _build_benchmark_data(n_bars=50, n_assets=5) diff --git a/tests/test_broker.py b/tests/test_broker.py index deaa150f..0c7bdbc6 100644 --- a/tests/test_broker.py +++ b/tests/test_broker.py @@ -1554,6 +1554,26 @@ def test_signal_exit_reason_is_present_on_fill_and_trade(self): assert broker.fills[-1].exit_reason == "signal" assert broker.trades[-1].exit_reason == "signal" + def test_reversal_fill_records_signal_exit_reason(self): + broker = Broker( + 100000.0, + NoCommission(), + NoSlippage(), + allow_short_selling=True, + allow_leverage=True, + ) + mark_prices(broker, {"AAPL": 100.0}) + broker.submit_order("AAPL", 10.0, OrderSide.BUY) + broker._process_orders() + + mark_prices(broker, {"AAPL": 105.0}) + broker.submit_order("AAPL", 15.0, OrderSide.SELL) + broker._process_orders() + + assert broker.fills[-1].exit_reason == "signal" + assert broker.trades[-1].exit_reason == "signal" + assert broker.get_position("AAPL").quantity == -5.0 + def test_evaluate_position_rules_exit_full_deferred(self): """Test EXIT_FULL action with defer_fill=True (NEXT_BAR_OPEN mode).""" from ml4t.backtest.risk.position.static import StopLoss diff --git a/tests/test_pre_risk_strategy.py b/tests/test_pre_risk_strategy.py index 35383e70..c7921bf4 100644 --- a/tests/test_pre_risk_strategy.py +++ b/tests/test_pre_risk_strategy.py @@ -10,6 +10,7 @@ DataFeed, Engine, ExecutionMode, + OrderType, StopLoss, Strategy, TrailingStop, @@ -60,7 +61,7 @@ def _record(self, phase: str, timestamp: datetime, broker: Broker) -> None: def on_before_risk(self, timestamp, data, context, broker) -> None: self._record("before_risk", timestamp, broker) - if broker.get_position("SPY") is None: + if broker.get_position("SPY") is None and not broker.get_pending_orders("SPY"): broker.submit_order("SPY", 10) def on_data(self, timestamp, data, context, broker) -> None: @@ -77,6 +78,33 @@ def on_data(self, timestamp, data, context, broker) -> None: pass +class PendingAwareLimitEntry(Strategy): + """Keep one untriggered limit order while the position remains flat.""" + + def on_before_risk(self, timestamp, data, context, broker) -> None: + if broker.get_position("SPY") is None and not broker.get_pending_orders("SPY"): + broker.submit_order( + "SPY", + 10, + order_type=OrderType.LIMIT, + limit_price=50.0, + ) + + def on_data(self, timestamp, data, context, broker) -> None: + pass + + +class ExitFundedEntry(Strategy): + """Exit one asset under risk before funding a prior-bar entry in another.""" + + def on_data(self, timestamp, data, context, broker) -> None: + if timestamp.day == 3: + broker.set_position_rules(StopLoss(pct=0.05), asset="AAPL") + broker.submit_order("AAPL", 90) + elif timestamp.day == 4: + broker.submit_order("GOOGL", 90) + + def _daily_prices(days: int = 3) -> pl.DataFrame: start = datetime(2026, 8, 3) timestamps = [start + timedelta(days=offset) for offset in range(days)] @@ -242,3 +270,103 @@ def test_next_bar_pre_risk_allows_explicit_pyramiding() -> None: assert sum(fill.quantity for fill in result.fills) == 20.0 assert engine.broker.get_position("SPY").quantity == 20.0 assert len(engine.broker.get_pending_orders("SPY")) == 1 + + +def test_next_bar_pre_risk_does_not_evaluate_new_position_until_following_bar() -> None: + prices = pl.DataFrame( + { + "timestamp": [datetime(2026, 8, day) for day in (3, 4, 5)], + "asset": ["SPY"] * 3, + "open": [100.0] * 3, + "high": [101.0] * 3, + "low": [99.0, 94.0, 94.0], + "close": [100.0] * 3, + "volume": [1_000_000.0] * 3, + } + ) + + result = Engine( + DataFeed(prices_df=prices), + OpeningTargetWithStop(), + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ).run() + + assert [(fill.side.value, fill.timestamp.day) for fill in result.fills] == [ + ("buy", 4), + ("sell", 5), + ] + + +def test_next_bar_pre_risk_excludes_entry_bar_extreme_from_trailing_watermark() -> None: + prices = pl.DataFrame( + { + "timestamp": [datetime(2026, 8, day) for day in (3, 4, 5)], + "asset": ["SPY"] * 3, + "open": [100.0] * 3, + "high": [100.0, 200.0, 100.0], + "low": [100.0, 95.0, 96.0], + "close": [100.0] * 3, + "volume": [1_000_000.0] * 3, + } + ) + engine = Engine( + DataFeed(prices_df=prices), + OpeningTargetWithTrailingStop(), + BacktestConfig( + execution_mode=ExecutionMode.NEXT_BAR, + trail_hwm_source=WaterMarkSource.BAR_EXTREME, + ), + ) + + result = engine.run() + + assert [(fill.side.value, fill.timestamp.day) for fill in result.fills] == [("buy", 4)] + assert engine.broker.get_position("SPY").high_water_mark == 100.0 + + +def test_next_bar_exit_first_preserves_exit_funded_entry() -> None: + rows = [] + for day in (3, 4, 5): + for asset in ("AAPL", "GOOGL"): + low = 94.0 if day == 5 and asset == "AAPL" else 100.0 + rows.append( + { + "timestamp": datetime(2026, 8, day), + "asset": asset, + "open": 100.0, + "high": 100.0, + "low": low, + "close": 100.0, + "volume": 1_000_000.0, + } + ) + + result = Engine( + DataFeed(prices_df=pl.DataFrame(rows)), + ExitFundedEntry(), + BacktestConfig( + initial_cash=10_000.0, + execution_mode=ExecutionMode.NEXT_BAR, + ), + ).run() + + assert result.rejected_orders == [] + assert [(fill.asset, fill.side.value, fill.timestamp.day) for fill in result.fills] == [ + ("AAPL", "buy", 4), + ("AAPL", "sell", 5), + ("GOOGL", "buy", 5), + ] + + +def test_next_bar_pending_limit_guard_does_not_duplicate_intent() -> None: + engine = Engine( + DataFeed(prices_df=_daily_prices()), + PendingAwareLimitEntry(), + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ) + + result = engine.run() + + assert result.fills == [] + assert len(engine.broker.orders) == 1 + assert len(engine.broker.get_pending_orders("SPY")) == 1 diff --git a/tests/test_rejected_order_results.py b/tests/test_rejected_order_results.py index 640b1798..74d352bb 100644 --- a/tests/test_rejected_order_results.py +++ b/tests/test_rejected_order_results.py @@ -8,7 +8,7 @@ from ml4t.backtest import BacktestConfig, Order, Strategy, run_backtest from ml4t.backtest.config import ShareType from ml4t.backtest.execution.limits import VolumeParticipationLimit -from ml4t.backtest.types import ExecutionMode, OrderSide, OrderStatus +from ml4t.backtest.types import ExecutionMode, OrderSide, OrderStatus, OrderType class _UnaffordableOrder(Strategy): @@ -43,6 +43,17 @@ def on_data(self, timestamp, data, context, broker) -> None: self.order = broker.submit_order("AAPL", self.quantity) +class _DelayedLimitOrder(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + if not broker.orders: + broker.submit_order( + "AAPL", + 10.0, + order_type=OrderType.LIMIT, + limit_price=50.0, + ) + + def _prices() -> pl.DataFrame: return pl.DataFrame( { @@ -120,6 +131,50 @@ def test_cash_account_short_rejection_has_structured_restriction_code() -> None: assert rejected.rejection_code == "account_restriction" +def test_margin_rejection_has_structured_buying_power_code() -> None: + result = run_backtest( + prices=_prices(), + strategy=_UnaffordableOrder(), + config=BacktestConfig( + initial_cash=100.0, + execution_mode=ExecutionMode.SAME_BAR, + allow_leverage=True, + ), + ) + + assert len(result.rejected_orders) == 1 + assert result.rejected_orders[0]._rejection_code == "insufficient_buying_power" + assert result.rejected_orders[0].rejection_code == "insufficient_buying_power" + + +def test_next_bar_shadow_queue_rejection_has_structured_cash_code() -> None: + prices = pl.DataFrame( + { + "timestamp": [datetime(2024, 1, day) for day in (2, 3, 4)], + "asset": ["AAPL"] * 3, + "open": [100.0, 100.0, 50.0], + "high": [100.0, 100.0, 50.0], + "low": [100.0, 100.0, 50.0], + "close": [100.0, 100.0, 50.0], + "volume": [1_000_000.0] * 3, + } + ) + result = run_backtest( + prices=prices, + strategy=_DelayedLimitOrder(), + config=BacktestConfig( + initial_cash=100.0, + execution_mode=ExecutionMode.NEXT_BAR, + next_bar_queue_shadow_validation=True, + ), + ) + + assert len(result.rejected_orders) == 1 + rejected = result.rejected_orders[0] + assert rejected.rejection_code == "insufficient_cash" + assert rejected._rejection_code == "insufficient_cash" + + def test_rejected_orders_round_trip_through_result_artifact(tmp_path) -> None: result = _run(_UnaffordableOrder()) diff --git a/tests/test_result.py b/tests/test_result.py index 2b4cfec4..b09fbbc9 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -805,7 +805,7 @@ def test_from_parquet_roundtrip(self, backtest_result: BacktestResult): with open(path / "manifest.json") as file: manifest = json.load(file) assert manifest["artifact_type"] == "ml4t-backtest-result" - assert manifest["schema_version"] == 1 + assert manifest["schema_version"] == 2 assert set(manifest["components"]) >= { "trades", "fills", @@ -889,6 +889,25 @@ def test_from_parquet_rejects_corrupt_daily_pnl(self, backtest_result: BacktestR with pytest.raises(ArtifactReadError, match="daily_pnl.parquet"): BacktestResult.from_parquet(path) + def test_from_parquet_rejects_daily_pnl_inconsistent_with_equity( + self, backtest_result: BacktestResult + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + daily_path = path / "daily_pnl.parquet" + daily = pl.read_parquet(daily_path).with_columns((pl.col("pnl") + 1.0).alias("pnl")) + daily.write_parquet(daily_path) + + with pytest.raises(ArtifactReadError, match="inconsistent"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert any( + diagnostic.code == "component_inconsistent" and diagnostic.component == "daily_pnl" + for diagnostic in recovered.artifact_diagnostics + ) + def test_from_parquet_rejects_unsupported_schema(self, backtest_result: BacktestResult): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" @@ -901,11 +920,6 @@ def test_from_parquet_rejects_unsupported_schema(self, backtest_result: Backtest with pytest.raises(UnsupportedArtifactVersionError, match="999"): BacktestResult.from_parquet(path) - manifest["artifact_type"] = "foreign-result" - manifest_path.write_text(json.dumps(manifest)) - with pytest.raises(UnsupportedArtifactVersionError, match="999"): - BacktestResult.from_parquet(path, recovery=True) - def test_foreign_artifact_type_fails_strict_and_recovers_current_schema( self, backtest_result: BacktestResult ): @@ -923,6 +937,21 @@ def test_foreign_artifact_type_fails_strict_and_recovers_current_schema( recovered = BacktestResult.from_parquet(path, recovery=True) assert recovered.artifact_diagnostics[0].code == "manifest_invalid" + def test_foreign_manifest_without_schema_can_recover_legacy_components( + self, backtest_result: BacktestResult + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / "manifest.json").write_text(json.dumps({"artifact_type": "foreign-result"})) + + with pytest.raises(ArtifactManifestError, match="foreign-result"): + BacktestResult.from_parquet(path) + + recovered = BacktestResult.from_parquet(path, recovery=True) + assert recovered.artifact_diagnostics[0].code == "manifest_invalid" + assert len(recovered.trades) == len(backtest_result.trades) + def test_noncanonical_manifest_component_fails_strict_and_recovers( self, backtest_result: BacktestResult ): @@ -1018,6 +1047,10 @@ def test_metrics_json_serialization(self, backtest_result: BacktestResult): assert metrics["array"] == [1.0, 2.0] assert metrics["series"] == [3.0, 4.0] + loaded = BacktestResult.from_parquet(path) + assert loaded.metrics["array"] == [1.0, 2.0] + assert loaded.metrics["series"] == [3.0, 4.0] + def test_nonfinite_metrics_use_portable_json_and_round_trip( self, backtest_result: BacktestResult ): @@ -1065,8 +1098,9 @@ def fail_write(*args, **kwargs): raise OSError("simulated component write failure") monkeypatch.setattr(pl.DataFrame, "write_parquet", fail_write) - with pytest.raises(OSError, match="simulated"): + with pytest.raises(ArtifactWriteError, match="trades component") as exc_info: backtest_result.to_parquet(path) + assert isinstance(exc_info.value.__cause__, OSError) assert not (path / "manifest.json").exists() assert (path / ".artifact-incomplete").exists() @@ -1087,6 +1121,20 @@ def test_metrics_json_rejects_unserializable_values(self): ): result.to_parquet(Path(tmpdir) / "test_backtest") + def test_metrics_json_rejects_array_class_with_metric_path(self): + result = BacktestResult( + trades=[], + equity_curve=[], + fills=[], + metrics={"opaque": pl.Series}, + ) + + with ( + tempfile.TemporaryDirectory() as tmpdir, + pytest.raises(ArtifactWriteError, match=r"metrics\['opaque'\]"), + ): + result.to_parquet(Path(tmpdir) / "test_backtest") + class TestEnrichTradesWithSignals: """Tests for enrich_trades_with_signals().""" diff --git a/tests/test_trade_mfe_mae.py b/tests/test_trade_mfe_mae.py index 8f3bef01..1dda522d 100644 --- a/tests/test_trade_mfe_mae.py +++ b/tests/test_trade_mfe_mae.py @@ -1,5 +1,6 @@ """Tests for MFE/MAE preservation in Trade class.""" +import math from datetime import datetime import pytest @@ -136,6 +137,18 @@ def test_empty_trades(self): assert analyzer.mfe_capture_ratio == 0.0 assert analyzer.mae_recovery_ratio == 0.0 + def test_partial_only_trades_report_unmeasured_lifecycle_metrics(self, sample_trades): + partial = sample_trades[0] + partial.status = "partial" + analyzer = TradeAnalyzer([partial]) + + assert analyzer.num_trades == 1 + assert math.isnan(analyzer.avg_bars_held) + assert math.isnan(analyzer.avg_mfe) + assert math.isnan(analyzer.avg_mae) + assert math.isnan(analyzer.mfe_capture_ratio) + assert math.isnan(analyzer.mae_recovery_ratio) + def test_to_dict_includes_mfe_mae(self, sample_trades): """Test that to_dict includes MFE/MAE metrics.""" analyzer = TradeAnalyzer(sample_trades) From dae5765731a13689f667b9be6db8ed04709a5618 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 15:31:26 -0400 Subject: [PATCH 14/15] fix: close final milestone review edge cases --- docs/user-guide/execution-semantics.md | 12 +- src/ml4t/backtest/analytics/trades.py | 3 +- src/ml4t/backtest/broker.py | 2 + src/ml4t/backtest/core/execution_engine.py | 94 +++++++++--- src/ml4t/backtest/core/order_book.py | 10 +- src/ml4t/backtest/engine.py | 10 +- src/ml4t/backtest/result.py | 23 ++- src/ml4t/backtest/strategy.py | 2 +- tests/test_config_wiring.py | 2 + tests/test_datafeed_memory.py | 1 - tests/test_pre_risk_strategy.py | 157 ++++++++++++++++++++- tests/test_result.py | 16 +++ 12 files changed, 286 insertions(+), 46 deletions(-) diff --git a/docs/user-guide/execution-semantics.md b/docs/user-guide/execution-semantics.md index 1ed6bc39..27ac5ecb 100644 --- a/docs/user-guide/execution-semantics.md +++ b/docs/user-guide/execution-semantics.md @@ -32,12 +32,12 @@ position rules are evaluated. The state visible to the callback depends on execu | Mode | Positions visible to `on_before_risk()` | Ordinary orders submitted there | |------|------------------------------------------|----------------------------------| -| `NEXT_BAR` | Includes marketable flat-position entries previously submitted by this callback | Pending until the next bar | +| `NEXT_BAR` | All open positions, plus market flat-position entries this callback submitted on a prior bar | Pending until the next bar | | `SAME_BAR` | State before regular pending-order processing | Processed during the current bar | In `SAME_BAR`, set `immediate_fill=True` when a position opened in `on_before_risk()` must receive stop or trailing-rule evaluation on that same bar. In `NEXT_BAR`, newly opened positions start risk -evaluation on the following bar, matching ordinary next-bar entry timing. A marketable prior entry +evaluation on the following bar, matching ordinary next-bar entry timing. A prior market entry fills before the callback. Limit and stop orders can remain pending, so a guarded entry checks both position and pending intent: @@ -47,9 +47,11 @@ def on_before_risk(self, timestamp, data, context, broker): broker.submit_order("SPY", 10) ``` -Orders submitted by `on_data()` retain the configured within-bar fill ordering with risk exits. -Explicit pyramiding remains available by submitting an additional order without the flat-position -and pending-order guard. +Orders submitted by `on_data()` retain the configured within-bar fill ordering with risk exits. A +pre-risk market entry that lacks buying power remains pending during the callback and +then participates in the normal ordered batch, so a same-bar exit can fund it. Limit and stop +orders stay in the normal ordered batch. Explicit pyramiding remains available by submitting an +additional order without the flat-position and pending-order guard. ### SAME_BAR diff --git a/src/ml4t/backtest/analytics/trades.py b/src/ml4t/backtest/analytics/trades.py index 15a3f022..a70b3322 100644 --- a/src/ml4t/backtest/analytics/trades.py +++ b/src/ml4t/backtest/analytics/trades.py @@ -129,8 +129,7 @@ def avg_bars_held(self) -> float: """Average bars held across fully closed position lifecycles.""" if not self._lifecycle_trades: return float("nan") if self.trades else 0.0 - bars = [t.bars_held for t in self._lifecycle_trades if hasattr(t, "bars_held")] - return float(np.mean(bars)) if bars else 0.0 + return float(np.mean([trade.bars_held for trade in self._lifecycle_trades])) @property def total_fees(self) -> float: diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 6f80a8e9..d112122c 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -1772,6 +1772,7 @@ def _process_orders( order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, only_pre_risk_flat_entries: bool = False, + defer_policy_rejections: bool = False, ): """Process pending orders against current prices. @@ -1791,4 +1792,5 @@ def _process_orders( order_types=order_types, include_orders_this_bar=include_orders_this_bar, only_pre_risk_flat_entries=only_pre_risk_flat_entries, + defer_policy_rejections=defer_policy_rejections, ) diff --git a/src/ml4t/backtest/core/execution_engine.py b/src/ml4t/backtest/core/execution_engine.py index 7fafa6af..4e8888b9 100644 --- a/src/ml4t/backtest/core/execution_engine.py +++ b/src/ml4t/backtest/core/execution_engine.py @@ -22,6 +22,7 @@ def process_orders( order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, only_pre_risk_flat_entries: bool = False, + defer_policy_rejections: bool = False, ): if self._should_use_next_bar_queue_shadow_validation( use_open, @@ -29,7 +30,13 @@ def process_orders( include_orders_this_bar=include_orders_this_bar, only_pre_risk_flat_entries=only_pre_risk_flat_entries, ): - self._process_orders_next_bar_queue_shadow(use_open) + self._process_orders_next_bar_queue_shadow( + use_open, + order_types=order_types, + include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, + defer_policy_rejections=defer_policy_rejections, + ) return ordering = self.broker.fill_ordering.value @@ -39,6 +46,7 @@ def process_orders( order_types=order_types, include_orders_this_bar=include_orders_this_bar, only_pre_risk_flat_entries=only_pre_risk_flat_entries, + defer_policy_rejections=defer_policy_rejections, ) elif ordering == "sequential": self._process_orders_sequential( @@ -46,6 +54,7 @@ def process_orders( order_types=order_types, include_orders_this_bar=include_orders_this_bar, only_pre_risk_flat_entries=only_pre_risk_flat_entries, + defer_policy_rejections=defer_policy_rejections, ) else: self._process_orders_fifo( @@ -53,6 +62,7 @@ def process_orders( order_types=order_types, include_orders_this_bar=include_orders_this_bar, only_pre_risk_flat_entries=only_pre_risk_flat_entries, + defer_policy_rejections=defer_policy_rejections, ) def _is_exit_order(self, order) -> bool: @@ -66,6 +76,7 @@ def _process_orders_exit_first( order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, only_pre_risk_flat_entries: bool = False, + defer_policy_rejections: bool = False, ): broker = self.broker fill = broker._fill_engine @@ -124,7 +135,12 @@ def _process_orders_exit_first( entry_orders = self._sort_entry_orders(entry_orders, use_open=use_open) for order in entry_orders: - self._process_single_order(order, use_open, filled_orders) + self._process_single_order( + order, + use_open, + filled_orders, + defer_policy_rejections=defer_policy_rejections, + ) self._cleanup_filled_orders(filled_orders) @@ -141,34 +157,41 @@ def _should_use_next_bar_queue_shadow_validation( use_open and broker.execution_mode is ExecutionMode.NEXT_BAR and broker.next_bar_queue_shadow_validation - and not only_pre_risk_flat_entries ): return False - if order_types is not None or include_orders_this_bar: + if (order_types is not None or include_orders_this_bar) and not only_pre_risk_flat_entries: return False current_bar_index = broker._bar_index - for order in broker.pending_orders: - if order.order_id in broker._orders_this_bar_ids: - continue + eligible_orders = self._eligible_orders( + use_open, + order_types=order_types, + include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, + ) + for order in eligible_orders: if getattr(order, "_created_bar_index", current_bar_index) < current_bar_index - 1: return True return False - def _process_orders_next_bar_queue_shadow(self, use_open: bool = False): + def _process_orders_next_bar_queue_shadow( + self, + use_open: bool = False, + *, + order_types: set[OrderType] | None = None, + include_orders_this_bar: bool = False, + only_pre_risk_flat_entries: bool = False, + defer_policy_rejections: bool = False, + ): broker = self.broker fill = broker._fill_engine - eligible_orders = [] - orders_this_bar_ids = broker._orders_this_bar_ids - - for order in broker.pending_orders[:]: - if ( - broker.execution_mode is ExecutionMode.NEXT_BAR - and order.order_id in orders_this_bar_ids - ): - continue - eligible_orders.append(order) + eligible_orders = self._eligible_orders( + use_open, + order_types=order_types, + include_orders_this_bar=include_orders_this_bar, + only_pre_risk_flat_entries=only_pre_risk_flat_entries, + ) if not eligible_orders: return @@ -210,6 +233,8 @@ def _process_orders_next_bar_queue_shadow(self, use_open: bool = False): shadow_positions=shadow_positions, ) if not valid: + if defer_policy_rejections: + continue order.reject( rejection_reason, rejection_code or "order_validation_failed", @@ -349,6 +374,7 @@ def _process_orders_fifo( order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, only_pre_risk_flat_entries: bool = False, + defer_policy_rejections: bool = False, ): broker = self.broker eligible_orders = self._eligible_orders( @@ -361,7 +387,12 @@ def _process_orders_fifo( filled_orders: list = [] for order in eligible_orders: - self._process_single_order(order, use_open, filled_orders) + self._process_single_order( + order, + use_open, + filled_orders, + defer_policy_rejections=defer_policy_rejections, + ) if filled_orders and filled_orders[-1] is order: broker.mark_account_positions(use_open=use_open) @@ -374,6 +405,7 @@ def _process_orders_sequential( order_types: set[OrderType] | None = None, include_orders_this_bar: bool = False, only_pre_risk_flat_entries: bool = False, + defer_policy_rejections: bool = False, ): """Process orders in submission order without exit/entry separation. @@ -433,7 +465,12 @@ def _process_orders_sequential( fill.update_partial_order(order) else: # No shadow validation — use full gatekeeper path - self._process_single_order(order, use_open, filled_orders) + self._process_single_order( + order, + use_open, + filled_orders, + defer_policy_rejections=defer_policy_rejections, + ) # Mark-to-market after every fill so the next order sees updated cash if filled_orders and filled_orders[-1] is order: @@ -441,7 +478,14 @@ def _process_orders_sequential( self._cleanup_filled_orders(filled_orders) - def _process_single_order(self, order, use_open: bool, filled_orders: list) -> None: + def _process_single_order( + self, + order, + use_open: bool, + filled_orders: list, + *, + defer_policy_rejections: bool = False, + ) -> None: broker = self.broker fill = broker._fill_engine if order.status is not OrderStatus.PENDING: @@ -504,6 +548,8 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N return if use_simple_cash_check and not self._passes_simple_cash_check(order, fill_price): + if defer_policy_rejections: + return order.reject("Insufficient cash (open cash check)", "insufficient_cash") return @@ -536,6 +582,8 @@ def _process_single_order(self, order, use_open: bool, filled_orders: list) -> N broker.gatekeeper.validate_order_with_code(order, fill_price) ) + if not valid and defer_policy_rejections: + return insufficient_cash = "insufficient" in rejection_reason.lower() if valid: fully_filled = fill.execute_fill(order, fill_price) @@ -596,7 +644,9 @@ def _eligible_orders( if order_types is not None and order.order_type not in order_types: continue if only_pre_risk_flat_entries and not ( - order._submitted_before_risk and order._submitted_from_flat + order._submitted_before_risk + and order._submitted_from_flat + and broker.get_position(order.asset) is None ): continue if ( diff --git a/src/ml4t/backtest/core/order_book.py b/src/ml4t/backtest/core/order_book.py index 8861853a..ba9dc008 100644 --- a/src/ml4t/backtest/core/order_book.py +++ b/src/ml4t/backtest/core/order_book.py @@ -529,8 +529,14 @@ def _passes_margin_submission_precheck(self, order: Order, signal_price: float) counts["accepted" if valid else "rejected"] += 1 if not valid: - order.rejection_reason = reason or "Insufficient buying power (submission precheck)" - order._rejection_code = broker.gatekeeper.classify_rejection(old_qty + size) + rejection_code = broker.gatekeeper.classify_rejection(old_qty + size) + fallback_reason = { + "account_restriction": "Account restriction (submission precheck)", + "insufficient_cash": "Insufficient cash (submission precheck)", + "insufficient_buying_power": "Insufficient buying power (submission precheck)", + }[rejection_code] + order.rejection_reason = reason or fallback_reason + order._rejection_code = rejection_code return False multiplier = broker.get_multiplier(order.asset) diff --git a/src/ml4t/backtest/engine.py b/src/ml4t/backtest/engine.py index a2bb1243..5af573c6 100644 --- a/src/ml4t/backtest/engine.py +++ b/src/ml4t/backtest/engine.py @@ -224,17 +224,19 @@ def run(self) -> BacktestResult: pre_risk_opened_assets: set[str] = set() if self.execution_mode == ExecutionMode.NEXT_BAR: - # Fill only flat-position entries submitted by this callback on a - # prior bar. Ordinary orders retain the configured exit-first batch. + # Fill only prior market entries submitted from a flat position by + # this callback. Other orders retain the configured ordered batch. positions_before = set(self.broker.positions) self.broker._process_orders( use_open=True, + order_types={OrderType.MARKET}, only_pre_risk_flat_entries=True, + defer_policy_rejections=True, ) pre_risk_opened_assets = set(self.broker.positions) - positions_before - # Optional strategy phase for opening orders that must receive risk - # protection during the current bar. Existing strategies inherit a no-op. + # Optional strategy phase. SAME_BAR immediate fills can receive current-bar + # risk; NEXT_BAR entries retain next-bar risk timing. self.broker._submitting_before_risk = True try: self.strategy.on_before_risk(timestamp, assets_data, context, self.broker) diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 08fbe286..e0248766 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -1271,13 +1271,22 @@ def read_spec_config(component_path: Path): config = spec_config if daily_pnl is not None: - expected_daily_pnl = cls( - trades=[], - equity_curve=equity_curve, - fills=[], - metrics={}, - ).to_daily_pnl() - if not daily_pnl.equals(expected_daily_pnl): + if not equity_curve and not daily_pnl.is_empty(): + diagnostics.append( + ArtifactDiagnostic( + code="component_unverified", + component="daily_pnl", + message="daily_pnl.parquet could not be verified without equity.parquet", + ) + ) + else: + expected_daily_pnl = cls( + trades=[], + equity_curve=equity_curve, + fills=[], + metrics={}, + ).to_daily_pnl() + if equity_curve and not daily_pnl.equals(expected_daily_pnl): message = "daily_pnl.parquet is inconsistent with equity.parquet" if not recovery: raise ArtifactReadError(message) diff --git a/src/ml4t/backtest/strategy.py b/src/ml4t/backtest/strategy.py index 8b95dc0d..47599e89 100644 --- a/src/ml4t/backtest/strategy.py +++ b/src/ml4t/backtest/strategy.py @@ -19,7 +19,7 @@ def on_before_risk( """Run strategy logic immediately before current-bar position risk. The broker has registered the current bar's prices before this callback. - In ``NEXT_BAR`` mode, a marketable flat-position entry submitted by this + In ``NEXT_BAR`` mode, a market flat-position entry submitted by this callback on a prior bar fills at the current open before the callback runs. Newly opened positions start risk evaluation on the following bar, preserving next-bar timing. Untriggered limit or stop orders remain pending, so guarded diff --git a/tests/test_config_wiring.py b/tests/test_config_wiring.py index 2736fb96..2fca2715 100644 --- a/tests/test_config_wiring.py +++ b/tests/test_config_wiring.py @@ -235,6 +235,8 @@ def test_next_bar_submission_precheck_rejects_immediately(self): assert order is not None assert order.status.value == "rejected" assert "submission precheck" in (order.rejection_reason or "").lower() + assert "insufficient cash" in (order.rejection_reason or "").lower() + assert order.rejection_code == "insufficient_cash" assert len(broker.pending_orders) == 0 def test_next_bar_submission_precheck_uses_sequential_shadow_cash(self): diff --git a/tests/test_datafeed_memory.py b/tests/test_datafeed_memory.py index 423e2e0b..950c1818 100644 --- a/tests/test_datafeed_memory.py +++ b/tests/test_datafeed_memory.py @@ -189,7 +189,6 @@ def test_datafeed_mixed_slice_lengths_from_unsorted_input(self): assert set(rows[2][1]) == {"AAPL"} assert rows[2][1]["AAPL"]["close"] == 300.5 - @pytest.mark.benchmark def test_datafeed_memory_benchmark(self): """Benchmark memory usage for medium-scale dataset. diff --git a/tests/test_pre_risk_strategy.py b/tests/test_pre_risk_strategy.py index c7921bf4..9679b32a 100644 --- a/tests/test_pre_risk_strategy.py +++ b/tests/test_pre_risk_strategy.py @@ -19,7 +19,7 @@ class OpeningTargetWithStop(Strategy): - """Enter at the session open and protect the new position on that bar.""" + """Enter at the session open; SAME_BAR immediate mode protects the entry bar.""" def on_before_risk(self, timestamp, data, context, broker) -> None: if broker.get_position("SPY") is None: @@ -105,6 +105,65 @@ def on_data(self, timestamp, data, context, broker) -> None: broker.submit_order("GOOGL", 90) +class ExitFundedPreRiskEntry(Strategy): + """Keep an unaffordable pre-risk entry pending until a risk exit funds it.""" + + def on_before_risk(self, timestamp, data, context, broker) -> None: + if ( + timestamp.day == 4 + and broker.get_position("GOOGL") is None + and not broker.get_pending_orders("GOOGL") + ): + broker.submit_order("GOOGL", 90) + + def on_data(self, timestamp, data, context, broker) -> None: + if timestamp.day == 3: + broker.set_position_rules(StopLoss(pct=0.05), asset="AAPL") + broker.submit_order("AAPL", 90) + + +class FlatLimitThenOrdinaryMarket(Strategy): + """Open through on_data while an older pre-risk limit remains pending.""" + + def __init__(self) -> None: + self.visible_quantities: list[tuple[int, float]] = [] + + def on_before_risk(self, timestamp, data, context, broker) -> None: + position = broker.get_position("SPY") + self.visible_quantities.append( + (timestamp.day, 0.0 if position is None else position.quantity) + ) + if timestamp.day == 3: + broker.submit_order( + "SPY", + 10, + order_type=OrderType.LIMIT, + limit_price=50.0, + ) + + def on_data(self, timestamp, data, context, broker) -> None: + if timestamp.day == 3: + broker.submit_order("SPY", 10) + + +class LatePricePreRiskEntry(Strategy): + """Keep a pre-risk market order pending until its asset first has a price.""" + + def __init__(self) -> None: + self.visible_quantities: list[tuple[int, float]] = [] + + def on_before_risk(self, timestamp, data, context, broker) -> None: + position = broker.get_position("SPY") + self.visible_quantities.append( + (timestamp.day, 0.0 if position is None else position.quantity) + ) + if timestamp.day == 3: + broker.submit_order("SPY", 10) + + def on_data(self, timestamp, data, context, broker) -> None: + pass + + def _daily_prices(days: int = 3) -> pl.DataFrame: start = datetime(2026, 8, 3) timestamps = [start + timedelta(days=offset) for offset in range(days)] @@ -224,7 +283,10 @@ def test_next_bar_pre_risk_guard_sees_filled_open_order() -> None: result = Engine( DataFeed(prices_df=_daily_prices()), strategy, - BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + BacktestConfig( + execution_mode=ExecutionMode.NEXT_BAR, + next_bar_queue_shadow_validation=True, + ), ).run() assert [(fill.timestamp.day, fill.quantity) for fill in result.fills] == [(4, 10.0)] @@ -370,3 +432,94 @@ def test_next_bar_pending_limit_guard_does_not_duplicate_intent() -> None: assert result.fills == [] assert len(engine.broker.orders) == 1 assert len(engine.broker.get_pending_orders("SPY")) == 1 + + +def test_next_bar_pre_risk_entry_can_use_same_bar_exit_proceeds() -> None: + rows = [] + for day in (3, 4, 5): + for asset in ("AAPL", "GOOGL"): + low = 94.0 if day == 5 and asset == "AAPL" else 100.0 + rows.append( + { + "timestamp": datetime(2026, 8, day), + "asset": asset, + "open": 100.0, + "high": 100.0, + "low": low, + "close": 100.0, + "volume": 1_000_000.0, + } + ) + + engine = Engine( + DataFeed(prices_df=pl.DataFrame(rows)), + ExitFundedPreRiskEntry(), + BacktestConfig( + initial_cash=10_000.0, + execution_mode=ExecutionMode.NEXT_BAR, + ), + ) + result = engine.run() + + assert result.rejected_orders == [] + assert [(fill.asset, fill.side.value, fill.timestamp.day) for fill in result.fills] == [ + ("AAPL", "buy", 4), + ("AAPL", "sell", 5), + ("GOOGL", "buy", 5), + ] + assert engine.broker.get_position("GOOGL").quantity == 90.0 + + +def test_pre_risk_limit_is_rechecked_for_flatness_before_early_fill() -> None: + prices = pl.DataFrame( + { + "timestamp": [datetime(2026, 8, day) for day in (3, 4, 5)], + "asset": ["SPY"] * 3, + "open": [100.0, 100.0, 50.0], + "high": [100.0, 100.0, 50.0], + "low": [100.0, 100.0, 50.0], + "close": [100.0, 100.0, 50.0], + "volume": [1_000_000.0] * 3, + } + ) + strategy = FlatLimitThenOrdinaryMarket() + engine = Engine( + DataFeed(prices_df=prices), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + ) + + result = engine.run() + + assert strategy.visible_quantities == [(3, 0.0), (4, 0.0), (5, 10.0)] + assert [(fill.timestamp.day, fill.quantity) for fill in result.fills] == [ + (4, 10.0), + (5, 10.0), + ] + assert engine.broker.get_position("SPY").quantity == 20.0 + + +def test_aged_pre_risk_market_entry_uses_queue_shadow_validation() -> None: + prices = pl.DataFrame( + { + "timestamp": [datetime(2026, 8, day) for day in (3, 4, 5)], + "asset": ["AAPL", "AAPL", "SPY"], + "open": [100.0] * 3, + "high": [100.0] * 3, + "low": [100.0] * 3, + "close": [100.0] * 3, + "volume": [1_000_000.0] * 3, + } + ) + strategy = LatePricePreRiskEntry() + result = Engine( + DataFeed(prices_df=prices), + strategy, + BacktestConfig( + execution_mode=ExecutionMode.NEXT_BAR, + next_bar_queue_shadow_validation=True, + ), + ).run() + + assert [(fill.asset, fill.timestamp.day) for fill in result.fills] == [("SPY", 5)] + assert strategy.visible_quantities == [(3, 0.0), (4, 0.0), (5, 10.0)] diff --git a/tests/test_result.py b/tests/test_result.py index b09fbbc9..e9a181f8 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -908,6 +908,22 @@ def test_from_parquet_rejects_daily_pnl_inconsistent_with_equity( for diagnostic in recovered.artifact_diagnostics ) + def test_recovery_marks_daily_pnl_unverified_when_equity_is_missing( + self, backtest_result: BacktestResult + ): + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "test_backtest" + backtest_result.to_parquet(path) + (path / "equity.parquet").unlink() + + recovered = BacktestResult.from_parquet(path, recovery=True) + diagnostics = { + (diagnostic.code, diagnostic.component) + for diagnostic in recovered.artifact_diagnostics + } + assert ("component_unverified", "daily_pnl") in diagnostics + assert ("component_inconsistent", "daily_pnl") not in diagnostics + def test_from_parquet_rejects_unsupported_schema(self, backtest_result: BacktestResult): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "test_backtest" From a441b696e3f5d34e495831ededac294c55d96bd7 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Thu, 6 Aug 2026 15:41:14 -0400 Subject: [PATCH 15/15] fix: address milestone review follow-up --- docs/user-guide/execution-semantics.md | 7 +++--- src/ml4t/backtest/result.py | 12 ++++++++--- src/ml4t/backtest/strategy.py | 6 ++++-- tests/test_pre_risk_strategy.py | 30 ++++++++++++++++++++++++-- tests/test_result.py | 16 ++++++++++++++ 5 files changed, 61 insertions(+), 10 deletions(-) diff --git a/docs/user-guide/execution-semantics.md b/docs/user-guide/execution-semantics.md index 27ac5ecb..b3f3610a 100644 --- a/docs/user-guide/execution-semantics.md +++ b/docs/user-guide/execution-semantics.md @@ -32,14 +32,15 @@ position rules are evaluated. The state visible to the callback depends on execu | Mode | Positions visible to `on_before_risk()` | Ordinary orders submitted there | |------|------------------------------------------|----------------------------------| -| `NEXT_BAR` | All open positions, plus market flat-position entries this callback submitted on a prior bar | Pending until the next bar | +| `NEXT_BAR` | All open positions, plus any fills from priced, policy-valid prior market entries submitted by this callback | Pending until the next bar | | `SAME_BAR` | State before regular pending-order processing | Processed during the current bar | In `SAME_BAR`, set `immediate_fill=True` when a position opened in `on_before_risk()` must receive stop or trailing-rule evaluation on that same bar. In `NEXT_BAR`, newly opened positions start risk evaluation on the following bar, matching ordinary next-bar entry timing. A prior market entry -fills before the callback. Limit and stop orders can remain pending, so a guarded entry checks both -position and pending intent: +fills before the callback only when a current price is available and policy and execution limits +permit it. Partial fills are visible to the callback while the remaining quantity stays pending. +Limit and stop orders can remain pending, so a guarded entry checks both position and pending intent: ```python def on_before_risk(self, timestamp, data, context, broker): diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index e0248766..ef504c93 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -1084,13 +1084,19 @@ def discover_legacy_components() -> dict[str, str]: ) components.pop(name) + component_read_ok: dict[str, bool] = {} + def read_component(name: str, reader, default): filename = components.get(name) if filename is None: + component_read_ok[name] = False return default try: - return reader(path / filename) + value = reader(path / filename) + component_read_ok[name] = True + return value except Exception as exc: + component_read_ok[name] = False if not recovery: raise ArtifactReadError( f"Failed to read {filename}: {type(exc).__name__}: {exc}" @@ -1271,7 +1277,7 @@ def read_spec_config(component_path: Path): config = spec_config if daily_pnl is not None: - if not equity_curve and not daily_pnl.is_empty(): + if not component_read_ok["equity"]: diagnostics.append( ArtifactDiagnostic( code="component_unverified", @@ -1286,7 +1292,7 @@ def read_spec_config(component_path: Path): fills=[], metrics={}, ).to_daily_pnl() - if equity_curve and not daily_pnl.equals(expected_daily_pnl): + if component_read_ok["equity"] and not daily_pnl.equals(expected_daily_pnl): message = "daily_pnl.parquet is inconsistent with equity.parquet" if not recovery: raise ArtifactReadError(message) diff --git a/src/ml4t/backtest/strategy.py b/src/ml4t/backtest/strategy.py index 47599e89..e304ba9a 100644 --- a/src/ml4t/backtest/strategy.py +++ b/src/ml4t/backtest/strategy.py @@ -19,8 +19,10 @@ def on_before_risk( """Run strategy logic immediately before current-bar position risk. The broker has registered the current bar's prices before this callback. - In ``NEXT_BAR`` mode, a market flat-position entry submitted by this - callback on a prior bar fills at the current open before the callback runs. + In ``NEXT_BAR`` mode, a priced, policy-valid market entry submitted from a + flat position by this callback on a prior bar can fill at the current open + before the callback runs. Partial fills are visible while the remainder stays + pending. Newly opened positions start risk evaluation on the following bar, preserving next-bar timing. Untriggered limit or stop orders remain pending, so guarded entries must check both ``broker.get_position(asset)`` and diff --git a/tests/test_pre_risk_strategy.py b/tests/test_pre_risk_strategy.py index 9679b32a..037fa7d3 100644 --- a/tests/test_pre_risk_strategy.py +++ b/tests/test_pre_risk_strategy.py @@ -16,6 +16,7 @@ TrailingStop, ) from ml4t.backtest.config import ExecutionPrice, WaterMarkSource +from ml4t.backtest.execution.limits import VolumeParticipationLimit class OpeningTargetWithStop(Strategy): @@ -45,7 +46,8 @@ def on_data(self, timestamp, data, context, broker) -> None: class GuardedPreRiskEntry(Strategy): """Enter only while no position exists and record callback-visible state.""" - def __init__(self) -> None: + def __init__(self, quantity: float = 10.0) -> None: + self.quantity = quantity self.trace: list[tuple[str, int, float, int]] = [] def _record(self, phase: str, timestamp: datetime, broker: Broker) -> None: @@ -62,7 +64,7 @@ def _record(self, phase: str, timestamp: datetime, broker: Broker) -> None: def on_before_risk(self, timestamp, data, context, broker) -> None: self._record("before_risk", timestamp, broker) if broker.get_position("SPY") is None and not broker.get_pending_orders("SPY"): - broker.submit_order("SPY", 10) + broker.submit_order("SPY", self.quantity) def on_data(self, timestamp, data, context, broker) -> None: self._record("on_data", timestamp, broker) @@ -523,3 +525,27 @@ def test_aged_pre_risk_market_entry_uses_queue_shadow_validation() -> None: assert [(fill.asset, fill.timestamp.day) for fill in result.fills] == [("SPY", 5)] assert strategy.visible_quantities == [(3, 0.0), (4, 0.0), (5, 10.0)] + + +def test_partially_filled_pre_risk_market_order_is_not_drained_before_risk() -> None: + prices = _daily_prices().with_columns(pl.lit(20.0).alias("volume")) + strategy = GuardedPreRiskEntry(quantity=20.0) + result = Engine( + DataFeed(prices_df=prices), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + execution_limits=VolumeParticipationLimit(max_participation=0.5), + ).run() + + assert [(fill.timestamp.day, fill.quantity) for fill in result.fills] == [ + (4, 10.0), + (5, 10.0), + ] + assert strategy.trace == [ + ("before_risk", 3, 0.0, 0), + ("on_data", 3, 0.0, 1), + ("before_risk", 4, 10.0, 1), + ("on_data", 4, 10.0, 1), + ("before_risk", 5, 10.0, 1), + ("on_data", 5, 20.0, 0), + ] diff --git a/tests/test_result.py b/tests/test_result.py index e9a181f8..5ff4a18a 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -908,6 +908,22 @@ def test_from_parquet_rejects_daily_pnl_inconsistent_with_equity( for diagnostic in recovered.artifact_diagnostics ) + def test_from_parquet_compares_successfully_read_empty_equity(self, tmp_path: Path): + result = BacktestResult( + trades=[], + equity_curve=[(datetime(2024, 1, 1), 100.0)], + fills=[], + metrics={}, + ) + path = tmp_path / "empty-equity" + result.to_parquet(path) + pl.DataFrame(schema={"timestamp": pl.Datetime, "equity": pl.Float64}).write_parquet( + path / "equity.parquet" + ) + + with pytest.raises(ArtifactReadError, match="inconsistent"): + BacktestResult.from_parquet(path) + def test_recovery_marks_daily_pnl_unverified_when_equity_is_missing( self, backtest_result: BacktestResult ):