Persist Firstrade strategy run state#16
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: 2a88d121f0
ℹ️ 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".
| except Exception as exc: | ||
| result["strategy_run_persisted"] = False | ||
| result["strategy_run_persistence_error"] = f"{type(exc).__name__}: {exc}" |
There was a problem hiding this comment.
The duplicate-run guard depends entirely on reading a terminal persisted stage (is_duplicate_live_run), but this except swallows persistence failures after execution. In a live cycle where orders were submitted, a transient GCS write failure here leaves the latest state as ORDERS_PLANNED (or missing), so the next invocation for the same month will not trip idempotency and can submit the same orders again. For live trading, returning success after failing to persist the terminal state breaks the duplicate-submission protection this change is adding.
Useful? React with 👍 / 👎.
| result["strategy_run_persistence_error"] = strategy_run_persistence_error | ||
| if persist_strategy_runs: | ||
| completed_state = build_strategy_run_state( | ||
| stage="DRY_RUN_COMPLETED" if settings.dry_run_only else "SUBMITTED", |
There was a problem hiding this comment.
This sets stage to SUBMITTED for every non-dry-run cycle, even when execution_result.action_done is false and no orders were actually submitted. Because duplicate detection treats SUBMITTED as terminal, a no-op live run (for example, all deltas below threshold) will block later runs in the same month that might legitimately need to submit orders after portfolio/market changes. The persisted stage should reflect whether a submission actually occurred.
Useful? React with 👍 / 👎.
1 participant
Summary
Tests