Use shared cash sweep whole-share helper#55
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8483f24469
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| funding_needs, | ||
| ) | ||
| ): | ||
| return int(max_quantity) |
There was a problem hiding this comment.
Preserve minimal sweep-sale quantity instead of liquidating max
cash_sweep_sale_quantity_to_fund_buy now converts a boolean decision into int(max_quantity), which sells the entire allowed BOXX amount whenever any funding gap exists. This is a regression from the prior quantity estimator and can materially over-liquidate cash sweep (especially in the fallback path where max_quantity is the full position), leaving large idle cash balances and suppressing the later sweep rebuy path because cash_sweep_sold_this_cycle becomes true.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1c84412c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| funding_needs, | ||
| ) | ||
| ): | ||
| return int(max_quantity) |
There was a problem hiding this comment.
Preserve computed sweep-sale size instead of liquidating max
This converts a boolean helper result into int(max_quantity), so any positive funding decision now sells the entire allowed BOXX block rather than the minimum shares needed to fund a whole-share buy; that can materially over-liquidate cash sweep and suppress same-cycle sweep rebuys because cash_sweep_sold_this_cycle flips true. Fresh evidence in this commit is that tests were updated to expect full liquidation amounts (for example 5/10 shares), confirming this regression is still present in the current patch.
Useful? React with 👍 / 👎.
| if current_buying_power >= quote_price: | ||
| return False |
There was a problem hiding this comment.
Continue scanning funding needs in fallback helper
In the compatibility fallback (used when quant_platform_kit cannot provide the new helper), hitting one candidate whose ask is already affordable immediately returns False, which prevents considering later underweight symbols that still require sweep cash; with multiple buy candidates, this can skip needed BOXX sells and leave downstream buys unfunded. The decision should continue evaluating remaining funding_needs rather than exiting on the first affordable symbol.
Useful? React with 👍 / 👎.
Switch Schwab cash-sweep decisions to the shared whole-share helper so BOXX is only sold when the full position can fund at least one whole-share buy.\n\nValidation:\n- pytest tests/test_rebalance_service.py -k 'refreshes_buying_power_after_sell_before_buying' -q\n- py_compile on touched files