Skip to content

fix: address review backlog#36

Merged
stefan-jansen merged 1 commit into
mainfrom
fix/review-backlog-cleanup
Jul 11, 2026
Merged

fix: address review backlog#36
stefan-jansen merged 1 commit into
mainfrom
fix/review-backlog-cleanup

Conversation

@stefan-jansen

Copy link
Copy Markdown
Contributor

Summary

  • wire risk liquidation through broker flattening and make liquidation updates idempotent
  • simplify affordability partial-fill logic by delegating cash validation to the gatekeeper
  • clean up stale docs-theme overrides, install docs, DataFeed timestamp indexing, and review-thread docstrings
  • add regression coverage for commission aliases, minimum-commission partial fills, DataFeed mixed slices, and liquidation flattening

Verification

  • pre-commit run --all-files
  • uv run ty check
  • uv run pytest tests/ -q
  • uv run mkdocs build --strict

Copilot AI review requested due to automatic review settings July 11, 2026 19:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a review backlog across the backtesting engine by tightening risk-liquidation behavior (broker flattening + idempotent application), simplifying partial-fill affordability via Gatekeeper validation, improving DataFeed timestamp indexing behavior, and cleaning up docs theme overrides—while adding regression tests around these areas.

Changes:

  • Route portfolio risk liquidations through Broker.flatten_all_positions() and make repeated liquidation updates idempotent (RiskManager).
  • Rework partial-fill affordability to delegate cash/commission validation to Gatekeeper.validate_order() (incl. minimum-commission coverage).
  • Normalize/strengthen DataFeed timestamp indexing and update docs/theme + installation instructions; add regression tests.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_datafeed_memory.py Adds regression coverage for signal-only timestamps and mixed slice-length iteration.
tests/test_config_wiring.py Adds tests for minimum-commission partial fills and per_contract commission alias; simplifies default-parity assertion.
tests/test_broker.py Refactors flattening test setup and adds a noop flattening regression test.
tests/risk/test_portfolio_manager.py Adds idempotent liquidation regression test and uses broker-driven position creation helpers.
src/ml4t/backtest/types.py Updates cost_drag docstring wording for clarity.
src/ml4t/backtest/risk/portfolio/manager.py Adds broker typing, idempotent liquidation guard, and resets guard on initialize/reset.
src/ml4t/backtest/result.py Documents and supports exporting spec.yaml via to_parquet(include=...).
src/ml4t/backtest/execution/rebalancer.py Updates FeedSpec typing import path and clarifies leverage/short weight semantics in docstrings.
src/ml4t/backtest/execution/fill_executor.py Adjusts slippage attribution for flipped positions to use per-unit slippage consistently.
src/ml4t/backtest/engine.py Makes OHLC fallbacks explicit per-asset using close/price base selection.
src/ml4t/backtest/datafeed.py Renames internal indices to entity terminology and replaces timestamp indexing implementation.
src/ml4t/backtest/core/shared.py Refines liquidation keyword matching via regex for exit-reason mapping.
src/ml4t/backtest/core/fill_engine.py Replaces affordability math with Gatekeeper-driven validation and bisection search.
src/ml4t/backtest/config.py Adds CommissionType._missing_ aliasing for "per_contract" and makes enum defaults use .value.
src/ml4t/backtest/accounting/policy.py Updates short-cash policy docs to include credit_proceeds.
mkdocs.yml Updates docs hub/library/chapter URLs to absolute production URLs.
docs/overrides/main.html Removes custom theme JS injection block.
docs/overrides/assets/stylesheets/ml4t-docs-theme.css Aligns overrides with updated MkDocs theme class names and comments.
docs/overrides/assets/javascripts/ml4t-docs-theme.js Removes stale theme JS file.
docs/getting-started/installation.md Updates clone instructions to the new repository path and trims dependency list.
Comments suppressed due to low confidence (1)

src/ml4t/backtest/datafeed.py:344

  • open/high/low currently fall back to close when those columns are missing, which leaves them as None when a feed provides price but no close. That propagates None values into the per-bar dict and can break downstream consumers that expect OHLC to be usable whenever price is present.
                close = row[price_close_idx] if price_close_idx >= 0 else None
                price = row[price_price_idx] if price_price_idx >= 0 else close
                open_ = row[price_open_idx] if price_open_idx >= 0 else close
                high = row[price_high_idx] if price_high_idx >= 0 else close
                low = row[price_low_idx] if price_low_idx >= 0 else close

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ml4t/backtest/core/fill_engine.py
@stefan-jansen stefan-jansen merged commit 4fc8449 into main Jul 11, 2026
8 checks passed
@stefan-jansen stefan-jansen deleted the fix/review-backlog-cleanup branch July 11, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants