Keep safe haven cash for unbuyable small accounts#45
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb3f6291e7
ℹ️ 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".
| safe_haven_symbols = tuple( | ||
| dict.fromkeys(str(symbol or "").strip().upper() for symbol in allocation.get("safe_haven_symbols", ())) | ||
| ) |
There was a problem hiding this comment.
When a plan identifies the sweep vehicle only via portfolio.cash_sweep_symbol and omits allocation.safe_haven_symbols, this new block never considers that symbol for the cash substitution. For example, with risk_symbols=("SOXX",), targets={"SOXX": 194, "BOXX": 1099}, and cash_sweep_symbol="BOXX", the unbuyable SOXX target is projected to zero but BOXX remains a positive target and the executor still sweeps the small account into BOXX. The earlier safe-haven helper already treats cash_sweep_symbol as a safe-haven symbol, so this path should use the same source.
Useful? React with 👍 / 👎.
| remaining_non_safe_targets = [ | ||
| symbol | ||
| for symbol in candidate_symbols | ||
| if float(adjusted_targets.get(symbol, 0.0) or 0.0) > 0.0 | ||
| ] |
There was a problem hiding this comment.
If risk_symbols/income_symbols arrive in a different case than the target keys, this lookup misses still-buyable non-safe targets after project_unbuyable_value_targets_to_cash normalizes target keys. In a mixed case plan such as risk_symbols=("soxl", "soxx") with uppercase targets where only SOXX is below one share, both lookups return zero here, so the code incorrectly treats all non-safe targets as gone and zeros the safe-haven target even though SOXL remains buyable. Normalize symbol for the adjusted_targets.get(...) check.
Useful? React with 👍 / 👎.
Pigbibi
force-pushed
the
fix-small-account-safe-haven-cash
branch
from
bb3f629 to
70030d8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70030d8d36
ℹ️ 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.
When FIRSTRADE_SAFE_HAVEN_CASH_SUBSTITUTE_THRESHOLD_USD is set to 0 (or otherwise below the BOXX/BIL target), the earlier helper intentionally leaves safe-haven targets unchanged, but this new branch still zeros them whenever a risk/income target was projected away and the adjusted total is <= $2,000. For example, with threshold 0, an unbuyable SOXX target and a $1,099.90 BOXX cash-sweep target, execution skips the BOXX buy anyway, so operators can no longer opt out of keeping safe-haven targets as cash via the documented setting.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification