Skip to content

chore: close remaining copilot cleanup items - #29

Merged
stefan-jansen merged 1 commit into
mainfrom
fix/remaining-copilot-cleanups
Jul 11, 2026
Merged

chore: close remaining copilot cleanup items#29
stefan-jansen merged 1 commit into
mainfrom
fix/remaining-copilot-cleanups

Conversation

@stefan-jansen

Copy link
Copy Markdown
Contributor

Summary

  • fix README clone directory command
  • use keyword arguments for the legacy _update_time test call
  • compute the insufficient-cash predicate once in execution handling

Verification

  • uv run pytest tests/test_broker.py::TestBrokerBasics::test_equity_uses_current_mark_prices tests/test_config_wiring.py::TestShareType::test_exit_first_classifies_after_integer_rounding -q
  • pre-commit run --files README.md src/ml4t/backtest/core/execution_engine.py tests/test_broker.py

Copilot AI review requested due to automatic review settings July 11, 2026 17:45

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 closes remaining cleanup items by improving documentation accuracy, making a legacy test call clearer/safer via keyword arguments, and refactoring order execution handling to avoid repeated “insufficient cash” predicate computation.

Changes:

  • Fix README clone instructions to cd backtest after cloning ml4t/backtest.
  • Update a legacy Broker._update_time(...) test call to use keyword arguments for cache inputs.
  • Compute the "insufficient" in rejection_reason.lower() predicate once in ExecutionEngine._process_single_order and reuse it across branches.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
README.md Corrects the post-clone directory name in the development quickstart commands.
tests/test_broker.py Switches _update_time invocation to explicit keyword args for cache fields (timestamp, prices, opens, highs, lows, volumes, signals).
src/ml4t/backtest/core/execution_engine.py Refactors execution flow to compute insufficient_cash once and reuse it for rejection/partial-fill branching.

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

@stefan-jansen
stefan-jansen merged commit afeb527 into main Jul 11, 2026
8 checks passed
@stefan-jansen
stefan-jansen deleted the fix/remaining-copilot-cleanups branch July 11, 2026 17:50
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