Surface IBKR gateway handshake timeouts separately#60
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5fb9d040
ℹ️ 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 base_buying_power >= required_buying_power: | ||
| return 0 |
There was a problem hiding this comment.
Continue checking all buy candidates before skipping cash sweep
The cash-sweep helper exits early when the first underweight symbol is affordable (base_buying_power >= required_buying_power), which prevents evaluating later symbols that may still be unfundable. In portfolios with multiple buy candidates, this can suppress the BOXX funding sell even though a subsequent candidate needs additional cash, leading to missed or undersized buys in the same rebalance cycle.
Useful? React with 👍 / 👎.
| snapshot_as_of="2026-03-31", | ||
| ), | ||
| dry_run_only=True, | ||
| dry_run_only=False, |
There was a problem hiding this comment.
Preserve dry-run flag for the structured summary test
This test now invokes execute_rebalance with dry_run_only=False, but the existing assertions in the same test still expect dry-run semantics (for example summary["mode"] == "dry_run"). As written, the test setup no longer matches its expectations and will fail in environments where the suite runs, reducing coverage for the summary contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 916b38db07
ℹ️ 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".
| position_quantity=positions.get(symbol, {}).get("quantity", 0), | ||
| quantity_step=order_quantity_step, | ||
| ) | ||
| qty = max(int(cash_sweep_quantity), int(regular_qty)) |
There was a problem hiding this comment.
Preserve quantity_step when sizing cash-sweep sells
The cash-sweep branch truncates both quantities to integers via max(int(cash_sweep_quantity), int(regular_qty)), which bypasses the step-aware sizing used elsewhere (_sell_order_quantity honors quantity_step). In accounts using fractional steps (e.g. 0.1) or non-unit steps, this can round a valid sell down to 0 or to an invalid size, so the BOXX funding sell is skipped/rejected and the subsequent buy remains underfunded.
Useful? React with 👍 / 👎.
Summary\n- Treat IBKR gateway handshake timeouts as a dedicated connection failure\n- Keep the timeout branch out of the generic strategy exception path\n- Add a standalone regression test for the new notification path\n\n## Verification\n- python -m py_compile main.py tests/test_connect_timeout_alert.py\n- python -m pytest tests/test_connect_timeout_alert.py -q