Order Management Overhaul #704

davidlatte · 2025-02-16T05:33:35Z

Advanced order handling of OCO/OTO/Bracket orders did not work in all contexts and brokers. There are also several confusing and deprecated features being utilized and all of these have been modified across all brokers so that the behavior is consistent going forward.

Change highlights:

order.type has been replaced with order.order_type to match (most) broker's terminology and to prevent shadowing with Python's built-in type.
order.order_type no longer accepts "oco", "oto", or "bracket". These have been consolidated into the order.order_class parameter for clarity.
order.child_orders is now populated automatically for all advanced order types at order creation time. Previously, each broker object was handling this themselves differently and it led to divergent and inconsistent behavior across brokers (including BackTestingBroker).
stop_limit and trailing_stop are now allowed within OCO/OTO/Bracket children. Previously these settings were ignored for any non-Simple order class.
Documentation changes to clarify how these options should be used going forward.
Backwards compatibility maintained with numerous checks and warnings are now issued to inform the user if their current methodology needs changing for the future.
Removed BackTestingBroker __get_attribute() silliness. Seriously silly ... seriously.

Why was this change made?

Tradier and BackTesting were not handling OCO orders in the same way. Each was making different assumptions that led to being unable for a single strategy to run using both. As part of trying to tease all of this out I kept coming upon more stumbling blocks and eventually had to just rip all of the order handling apart so I could put it back together in a way that will make sense for most brokers. The previous implementation was very much tied to IBKR specific ways of doing things that don't apply 1-to-1 with the more modern brokers.

How do make sense of all of this

Start with the changes made to strategy.create_order(). The documenation has been updated here to explain what the expected behavior is supposed to be.
Next look at Order.__init__(). It will show how backwards compatibility is maintained.
Look at Order._set_order_class_children. This should make intuitive sense based upon how OCO/OTO/Bracket orders work. Any assumptions that should be challenged will most likely come from looking at this code. This code will also lead you nicely down the rabbit hole of looking at the mess in all of the other Brokers.

Testing

All unit tests are currently passing (with the exception of the drift rebalancer, but I think that is unrelated)
I still need to do some long running backtesting and live Tradier testing, but that is going to have to wait until next week and I felt like this PR is already so big that it is ready from some other eyeballs.

Description by Korbit AI

What change is being made?

Refactor and enhance order management system for improved handling of complex orders in the Lumibot backtesting framework and associated broker implementations.

Why are these changes being made?

These changes address the need for a more comprehensive and flexible order management system within Lumibot, allowing for better support of advanced order types like OCO, Bracket, and OTO orders. The refactoring simplifies code and resolves existing limitations around order submission, modification, and execution logic, aligning closely with broker-specific requirements and improving backtesting accuracy for strategies that utilize complex orders. Additionally, the update standardizes the handling of order types and classes across different components, which was previously inconsistent and led to potential errors. This provides a more reliable foundation for both testing and live trading environments.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

… orders did not work in all contexts and brokers. There are also several confusing and deprecated features being utilized and all of these have been modified across all brokers so that the behavior is consistent going forward. Change highlights: - order.type has been replaced with order.order_type to match (most) broker's terminology and to prevent shadowing with Python's built-in 'type'. - order.order_type no longer accepts "oco", "oto", or "bracket". These have been consolidated into the order.order_class parameter for clarity. - order.child_orders is now populated automatically for all advanced order types at order creation time. Previously, each broker object was handling this themselves differently and it led to divergent and inconsistent behavior across brokers (including BackTestingBroker). - stop_limit and trailing_stop are now allowed within OCO/OTO/Bracket children. Previously these settings were ignored for any non-Simple order class. - Documentation changes to clarify how these options should be used going forward. - Backwards compatibility maintained with numerous checks and warnings are now issued to inform the user if their current methodology needs changing for the future. - Removed BackTestingBroker __get_attribute() silliness. Seriously silly ... seriously.

korbit-ai · 2025-02-16T05:33:40Z

Important

Required App Permission Update

Noise Reduction Improvements

This update requests write permissions for Commit Statuses in order to send updates directly to your PRs without adding comments that spam notifications. Visit our changelog to learn more.

Click here to accept the updated permissions

_{To accept the updated permissions, sufficient privileges are required}

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Fix Detected
	Incorrect Price Reference in Take Profit Orders ▹ view	✅
	Incorrect Enum Reference for Order Classes ▹ view	✅
	Invalid Portfolio Weight Distribution on Zero Total Value ▹ view

Files scanned

File Path	Reviewed
lumibot/example_strategies/stock_bracket.py	✅
lumibot/example_strategies/stock_oco.py	✅
lumibot/entities/asset.py	✅
lumibot/components/drift_rebalancer_logic.py	✅
lumibot/brokers/alpaca.py	✅
lumibot/brokers/ccxt.py	✅
lumibot/brokers/interactive_brokers_rest.py	✅
lumibot/brokers/broker.py	✅
lumibot/backtesting/backtesting_broker.py	✅
lumibot/brokers/tradier.py	✅
lumibot/brokers/interactive_brokers.py	✅
lumibot/entities/order.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

Chat with Korbit on issues we post by tagging @korbit-ai in your reply.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

Check out our docs on how you can make Korbit work best for you and your team.

Customize Korbit for your organization through the Korbit Console.

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ [email protected]

lumibot/brokers/alpaca.py

+                    if isinstance(order.limit_price, Decimal)
+                    else order.limit_price,


lumibot/brokers/alpaca.py

+        elif order.order_class in [Order.OrderType.BRACKET, Order.OrderType.OTO]:
+            alpaca_type = Order.OrderType.MARKET


korbit-ai · 2025-02-16T05:39:06Z

lumibot/components/drift_rebalancer_logic.py

        total_value = self.df["current_value"].sum()
-        self.df["current_weight"] = self.df["current_value"] / total_value
+        self.df["current_weight"] = self.df["current_value"] / total_value if total_value > 0 else Decimal(0)


Invalid Portfolio Weight Distribution on Zero Total Value

Tell me more

What is the issue?

The handling of zero total_value creates inconsistent portfolio weights. When total_value is zero, all current_weights are set to 0, which means the sum of portfolio weights will be 0 instead of 1.

Why this matters

This can lead to incorrect drift calculations and rebalancing decisions. In a valid portfolio, weights should always sum to 1 (or -1 for short positions), even when total_value is zero.

Suggested change ∙ Feature Preview

A better approach would be to use target weights when total_value is zero:

total_value = self.df["current_value"].sum() self.df["current_weight"] = ( self.df["current_value"] / total_value if total_value > 0 else self.df["target_weight"] )

_{💬 Chat with Korbit by mentioning @korbit-ai.}

…s orders.

…pdate that turned off Cash position reporting because AI helpers were getting confused.

brettelliot · 2025-02-19T00:44:07Z

lumibot/brokers/alpaca.py

@@ -355,9 +360,15 @@ def _parse_broker_order(self, response, strategy_name, strategy_object=None):
            ),
            Decimal(response.qty),
            response.side,
-            limit_price=response.limit_price,
+            limit_price=limit_price,  # order.py always converts to 'float'. Crypto issue?


yeah i would have assumed this was an issue. maybe it is and we don't know cause yahoo is down. i can test with alpaca crypto

i dont know why the test doesnt work as before. previous method retured a value, but now its returning a series and i had to extract the value. maybe pandas changed

… parent orders that are not technically submitted as a separate order. This is primarily for BackTesting as Tradier and other brokers do return it as an actual order.

…oco_stop_limit_deconflict # Conflicts: # tests/test_get_historical_prices.py

… valid strike instead of using current_asset_price.

davidlatte requested a review from grzesir as a code owner February 16, 2025 05:33

davidlatte had a problem deploying to unit-tests February 16, 2025 05:33 — with GitHub Actions Failure

davidlatte requested a review from brettelliot February 16, 2025 05:33

korbit-ai bot reviewed Feb 16, 2025

View reviewed changes

Type fix for alpaca code referencing wrong enum

5eb120c

davidlatte had a problem deploying to unit-tests February 17, 2025 23:27 — with GitHub Actions Failure

alpaca.py: missed the child_stop order settings for advanced stop los…

941dead

…s orders.

davidlatte had a problem deploying to unit-tests February 17, 2025 23:37 — with GitHub Actions Failure

grzesir approved these changes Feb 18, 2025

View reviewed changes

strategy: Adding option 'include_cash_positions' to work around the u…

a0fa90b

…pdate that turned off Cash position reporting because AI helpers were getting confused.

davidlatte had a problem deploying to unit-tests February 18, 2025 21:39 — with GitHub Actions Failure

brettelliot reviewed Feb 19, 2025

View reviewed changes

update yfinance; fix tests

aa1d6bf

i dont know why the test doesnt work as before. previous method retured a value, but now its returning a series and i had to extract the value. maybe pandas changed

brettelliot had a problem deploying to unit-tests February 19, 2025 11:52 — with GitHub Actions Failure

dont check the actual price. who knows why it can change

74254b6

brettelliot temporarily deployed to unit-tests February 19, 2025 11:59 — with GitHub Actions Inactive

include_cash_positions for example strategies

7d2ed8b

brettelliot temporarily deployed to unit-tests February 19, 2025 12:10 — with GitHub Actions Inactive

davidlatte added 3 commits February 20, 2025 17:17

Added "placeholder" order tracking so that we don't lose track of OCO…

394aa69

… parent orders that are not technically submitted as a separate order. This is primarily for BackTesting as Tradier and other brokers do return it as an actual order.

Merge remote-tracking branch 'origin/dev' into oco_stop_limit_deconflict

a16a2ae

Merge remote-tracking branch 'origin/oco_stop_limit_deconflict' into …

ab44a09

…oco_stop_limit_deconflict # Conflicts: # tests/test_get_historical_prices.py

davidlatte had a problem deploying to unit-tests February 21, 2025 01:20 — with GitHub Actions Failure

polygon: updated backtests for AMZN to use a chain lookup to select a…

e5e28e3

… valid strike instead of using current_asset_price.

davidlatte temporarily deployed to unit-tests February 21, 2025 23:59 — with GitHub Actions Inactive

davidlatte merged commit 9fc82d8 into dev Feb 22, 2025
1 check passed

davidlatte deleted the oco_stop_limit_deconflict branch February 22, 2025 00:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Order Management Overhaul #704

Order Management Overhaul #704

davidlatte commented Feb 16, 2025 •

edited by korbit-ai bot

Loading

korbit-ai bot commented Feb 16, 2025

Noise Reduction Improvements

korbit-ai bot left a comment •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

korbit-ai bot Feb 16, 2025

brettelliot Feb 19, 2025

		if isinstance(order.limit_price, Decimal)
		else order.limit_price,

		elif order.order_class in [Order.OrderType.BRACKET, Order.OrderType.OTO]:
		alpaca_type = Order.OrderType.MARKET

Order Management Overhaul #704

Order Management Overhaul #704

Conversation

davidlatte commented Feb 16, 2025 • edited by korbit-ai bot Loading

Change highlights:

Why was this change made?

How do make sense of all of this

Testing

Description by Korbit AI

What change is being made?

Why are these changes being made?

korbit-ai bot commented Feb 16, 2025

Noise Reduction Improvements

korbit-ai bot left a comment • edited Loading

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Interacting with Korbit

Customizing Korbit

Feedback and Support

This comment was marked as resolved.

This comment was marked as resolved.

korbit-ai bot Feb 16, 2025

Choose a reason for hiding this comment

Invalid Portfolio Weight Distribution on Zero Total Value

What is the issue?

Why this matters

Suggested change ∙ Feature Preview

brettelliot Feb 19, 2025

Choose a reason for hiding this comment

davidlatte commented Feb 16, 2025 •

edited by korbit-ai bot

Loading

korbit-ai bot left a comment •

edited

Loading