Keep safe haven cash for unbuyable small accounts#95
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3692604741
ℹ️ 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".
| not cash_sweep_sold_this_cycle | ||
| and cash_sweep_symbol | ||
| and cash_sweep_symbol in strategy_assets | ||
| and float(target_values.get(cash_sweep_symbol, 0.0) or 0.0) > 0.0 |
There was a problem hiding this comment.
Preserve cash-sweep rebuy when no explicit BOXX target
When a strategy relies on cash_sweep_symbol as the fallback sweep instrument but has a zero BOXX target, this new guard skips the final rebuy even if investable_cash is above the threshold and can buy multiple shares. That regresses the existing “remaining investable cash” sweep path (for example the test_dry_run_rebuys_cash_sweep_symbol_with_remaining_investable_cash scenario with BOXX target 0.0 and $1200 cash), leaving cash idle instead of buying BOXX; the condition needs to distinguish the small-account unbuyable-risk substitution from the normal cash-sweep rebuy case.
Useful? React with 👍 / 👎.
3d91da0 to
e574543
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e574543ba9
ℹ️ 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".
| if float(adjusted_targets.get(symbol, 0.0) or 0.0) > 0.0: | ||
| adjusted_targets[symbol] = 0.0 |
There was a problem hiding this comment.
Track cash substitution after threshold already zeroed BOXX
When _apply_safe_haven_cash_substitution has already zeroed a BOXX target below the safe-haven threshold (for example BOXX target $900 with a $1000 threshold), this check does not record small_account_safe_haven_cash_substituted_symbols even if all risk/income targets were just projected to cash. The final cash-sweep guard then treats this as a normal zero-target sweep case and can rebuy BOXX whenever investable_cash >= threshold, undoing the intended “keep cash for unbuyable small accounts” behavior for accounts whose safe-haven sleeve was under the threshold but cash is still above it.
Useful? React with 👍 / 👎.
e574543 to
d778e96
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d778e960cb
ℹ️ 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".
| if ( | ||
| substituted | ||
| and not remaining_non_safe_targets | ||
| and _positive_target_total(adjusted_targets) <= SMALL_ACCOUNT_SAFE_HAVEN_CASH_SUBSTITUTE_LIMIT_USD | ||
| ): |
There was a problem hiding this comment.
Count substituted targets in the small-account cap
When a risk/income target is projected to zero, this cap is checked against adjusted_targets, so the substituted sleeve no longer contributes to the $2,000 limit. For example, with a $1,900 risk target whose share price is $2,000 plus a $1,900 BOXX target, substituted is true and the adjusted total is only $1,900, causing BOXX to be zeroed and sold/blocked even though the strategy account was targeting $3,800; use the original target total or portfolio equity for the small-account limit.
Useful? React with 👍 / 👎.
Summary
Verification