Use confirmed sell release for Firstrade buys#219
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: The Codex review could not be completed. The review check fails closed until a valid Codex review is available. |
Co-Authored-By: Codex <noreply@openai.com>
![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: 011baa6f8f
ℹ️ 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".
| confirmed_sell_release_value = compute_confirmed_sell_release_value( | ||
| submitted_sell_orders=submitted_sell_orders, | ||
| fetch_order_status=fetch_order_status, |
There was a problem hiding this comment.
Here the release calculation receives the just-submitted sell order, so when get_order_status cannot find the order yet (common immediately after submitting a live limit sell, and also for preview/dry-run payloads), the pinned helper falls back to the submission payload/order dict. Firstrade submission payloads contain the requested shares/limit_price while still pending, which can be credited as released cash and allow the later buy loop to spend proceeds from an unfilled sell; only fetched fill data (or a status explicitly confirmed as filled) should be used for this value.
Useful? React with 👍 / 👎.
| "quantity", | ||
| "shares", | ||
| "qty", |
There was a problem hiding this comment.
Even when get_order_status does find an order-status row, these fallback aliases treat the order's requested quantity/shares as executed_qty. For a live sell that is still open or pending, Firstrade status rows commonly still include the requested share count, and the release helper accepts a positive executed quantity even without a confirmed fill status; this can again credit unfilled sell proceeds and submit buys against cash that has not settled into the account.
Useful? React with 👍 / 👎.
| safe_haven_cash_substitute_threshold_usd=settings.safe_haven_cash_substitute_threshold_usd, | ||
| cash_only_execution=settings.cash_only_execution, | ||
| notional_buy_execution=notional_buy_execution_enabled(settings.strategy_profile), | ||
| fetch_order_status=lambda broker_order_id: client.get_order_status(account, broker_order_id), |
There was a problem hiding this comment.
When client.get_order_status raises (for example, the Firstrade order-status endpoint times out or returns an unexpected payload), that exception bubbles out of execute_value_target_plan after sell orders may already have been submitted. In that scenario the run aborts before producing the submitted/skipped order summary or completed strategy-run state; the safer behavior for an unavailable status lookup is to treat the confirmed release as zero and skip dependent buys instead of crashing the cycle.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf81cca684
ℹ️ 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".
| elif float(delta_value) > 0.0 and limit_price > max(0.0, buy_budget): | ||
| requires_pending_sell_release = True | ||
| if estimated_buy_cost > investable_cash or requires_pending_sell_release: |
There was a problem hiding this comment.
When a negative delta floors to zero shares, this function still adds that symbol to pending_sell_release_symbols above even though no sell order was submitted. In that state, any later buy whose limit price is above its current budget sets requires_pending_sell_release here, and the condition below clears every buy_delta, so buys that were fully affordable from existing cash are skipped as pending_sell_release even though no sell can ever release funds. Limit this check to actually submitted pending sells, or skip only the buy that needs unconfirmed proceeds.
Useful? React with 👍 / 👎.
| for key, value in flattened.items(): | ||
| if _sanitize_payload_key(key.rsplit(".", 1)[-1]) in candidates: | ||
| return value |
There was a problem hiding this comment.
This helper receives candidates in priority order, but it returns whichever matching key appears first in Firstrade's payload. When an order-status row includes both a generic field such as description/limit_price and the real status/avg_price, an earlier generic key shadows the fill field, so confirmed fills can be treated as unfilled or released at the wrong price. Iterate candidate keys by priority instead of payload order so the explicit fill/status fields win.
Useful? React with 👍 / 👎.
1 participant
Summary
Dependency
Testing