Make the account summary 24 hours min requirement check more robust by using 24 hours minus 5 mins. #806
Conversation
… using 24 hours minus 5 mins.
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Unexplained Magic Numbers in Timing Logic ▹ view | 🧠 Not in scope |
Files scanned
| File Path | Reviewed |
|---|---|
| lumibot/strategies/_strategy.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| # Check if it has been at least 24 hours since the last account summary | ||
| if self.last_account_summary_dt is None or time_since_last_account_summary.total_seconds() >= 86400: # 24 hours | ||
| # Check if it has been at least 24 hours - 5mins since the last account summary | ||
| if self.last_account_summary_dt is None or time_since_last_account_summary.total_seconds() >= (86400 - 300): |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…implemented in BacktestingBroker.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Order Processing Skipped in Alpaca Broker
The self.process_pending_orders(strategy=strategy) call was accidentally removed from lumibot/brokers/alpaca.py. This change, unrelated to the PR's stated purpose of fixing account summary timing, prevents pending orders from being processed, breaking order execution functionality in the Alpaca broker.
lumibot/brokers/alpaca.py#L328-L331
lumibot/lumibot/brokers/alpaca.py
Lines 328 to 331 in 746fc56
Was this report helpful? Give feedback by reacting with 👍 or 👎
| @@ -326,9 +326,6 @@ def _await_market_to_close(self, timedelta=None, strategy=None): | |||
| The calling strategy; forwarded so pending orders can be processed | |||
| the same way BacktestingBroker does. | |||
| """ | |||
| # First, handle any orders waiting to be processed. | |||
| self.process_pending_orders(strategy=strategy) | |||
|
Hey I'm be happy to merge this but i think you need to add back process_pending_orders() |
Sometimes it won't send me the account summary in the morning because of the 24 hours min gap check failed.
Description by Korbit AI
What change is being made?
Update the minimum requirement check for sending the account summary to Discord, reducing the 24-hour threshold by 5 minutes.
Why are these changes being made?
The adjustment accounts for potential discrepancies in timing and ensures that minor delays or drifts in time do not prevent the account summary from being sent if it has been slightly less than 24 hours since the last summary. This enhances the robustness of the timing check and prevents unnecessary delays in communication.