-
Notifications
You must be signed in to change notification settings - Fork 0
fix: harden IBKR strategy switch contract #199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -407,6 +407,31 @@ jobs: | |
| print(f"- `{assignment['name']}` = `{assignment['value']}`") | ||
| PY | ||
|
|
||
| - name: Preflight IBKR deployment plan | ||
| if: env.PLATFORM == 'ibkr' && env.SERVICE_TARGETS_MODE != 'off' | ||
| env: | ||
| TARGET_REPOSITORY: ${{ steps.platform.outputs.repository }} | ||
| run: | | ||
| set -euo pipefail | ||
| platform_root="${RUNNER_TEMP}/interactive-brokers-platform" | ||
| repository_variables_file="${RUNNER_TEMP}/ibkr-repository-variables.json" | ||
| trap 'rm -f "${repository_variables_file}"' EXIT | ||
|
|
||
| gh auth setup-git | ||
| git clone --quiet --depth 1 --branch main \ | ||
| "https://github.com/${TARGET_REPOSITORY}.git" \ | ||
| "${platform_root}" | ||
| python3 -m pip install --quiet uv | ||
| uv sync --frozen --no-dev --quiet --project "${platform_root}" | ||
| gh variable list \ | ||
| --repo "${TARGET_REPOSITORY}" \ | ||
| --json name,value \ | ||
| > "${repository_variables_file}" | ||
|
Comment on lines
+426
to
+429
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an IBKR switch uses the supported Useful? React with 👍 / 👎. |
||
| python3 python/scripts/preflight_ibkr_switch.py \ | ||
| --target-file "${TARGET_FILE}" \ | ||
| --platform-root "${platform_root}" \ | ||
| --repository-variables-file "${repository_variables_file}" | ||
|
|
||
| - name: Apply GitHub variable updates | ||
| if: env.APPLY_SWITCH == 'true' | ||
| run: python3 python/scripts/runtime_settings.py apply "${TARGET_FILE}" --yes | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,27 +62,27 @@ | |
| "profiles": { | ||
| "us_daily": { | ||
| "timezone": "America/New_York", | ||
| "main_time": "45 15 * * *", | ||
| "probe_time": "35 9,15 * * *", | ||
| "precheck_time": "45 9 * * *" | ||
| "main_time": "45 15 * * 1-5", | ||
| "probe_time": "35 9,15 * * 1-5", | ||
| "precheck_time": "45 9 * * 1-5" | ||
| }, | ||
| "us_dca_month_end": { | ||
| "timezone": "America/New_York", | ||
| "main_time": "45 15 25-29 * *", | ||
| "probe_time": "35 9,15 25-29 * *", | ||
| "precheck_time": "45 9 25-29 * *" | ||
| "main_time": "45 15 * * 1-5", | ||
| "probe_time": "35 9,15 * * 1-5", | ||
| "precheck_time": "45 9 * * 1-5" | ||
| }, | ||
| "us_snapshot_month_start": { | ||
| "timezone": "America/New_York", | ||
| "main_time": "45 15 1-7 * *", | ||
| "probe_time": "35 9,15 1-7 * *", | ||
| "precheck_time": "45 9 1-7 * *" | ||
| "main_time": "45 15 * * 1-5", | ||
| "probe_time": "35 9,15 * * 1-5", | ||
| "precheck_time": "45 9 * * 1-5" | ||
| }, | ||
| "hk_daily": { | ||
| "timezone": "Asia/Hong_Kong", | ||
| "main_time": "45 15 * * *", | ||
| "probe_time": "35 9,15 * * *", | ||
| "precheck_time": "45 9 * * *" | ||
| "main_time": "45 15 * * 1-5", | ||
| "probe_time": "35 9,15 * * 1-5", | ||
| "precheck_time": "45 9 * * 1-5" | ||
| }, | ||
| "hk_snapshot_month_start": { | ||
| "timezone": "Asia/Hong_Kong", | ||
|
|
@@ -501,6 +501,13 @@ | |
| "dry_run" | ||
| ], | ||
| "blocked_live_reason": "", | ||
| "runtime_artifacts": { | ||
| "feature_snapshot": { | ||
| "required": true, | ||
| "path": "gs://qsl-runtime-logs-shared/strategy-artifacts/us_equity/global_etf_rotation/global_etf_rotation_feature_snapshot_latest.csv", | ||
| "manifest_path": "gs://qsl-runtime-logs-shared/strategy-artifacts/us_equity/global_etf_rotation/global_etf_rotation_feature_snapshot_latest.csv.manifest.json" | ||
| } | ||
| }, | ||
| "features": { | ||
| "income_layer": true, | ||
| "option_overlay": true, | ||
|
|
@@ -541,6 +548,13 @@ | |
| "dry_run" | ||
| ], | ||
| "blocked_live_reason": "", | ||
| "runtime_artifacts": { | ||
| "feature_snapshot": { | ||
| "required": true, | ||
| "path": "gs://qsl-runtime-logs-shared/strategy-artifacts/us_equity/russell_top50_leader_rotation_staging/russell_top50_leader_rotation_feature_snapshot_latest.csv", | ||
| "manifest_path": "gs://qsl-runtime-logs-shared/strategy-artifacts/us_equity/russell_top50_leader_rotation_staging/russell_top50_leader_rotation_feature_snapshot_latest.csv.manifest.json" | ||
| } | ||
| }, | ||
| "features": { | ||
| "income_layer": true, | ||
| "option_overlay": true, | ||
|
|
@@ -691,13 +705,17 @@ | |
| "scheduler_profile": "hk_snapshot_month_start", | ||
| "runtime_enabled": true, | ||
| "lifecycle_stage": "runtime_enabled", | ||
| "can_switch_live": true, | ||
| "can_switch_live": false, | ||
| "allowed_execution_modes": [ | ||
| "live", | ||
| "paper", | ||
| "dry_run" | ||
|
Comment on lines
709
to
711
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The catalog exposes Useful? React with 👍 / 👎. |
||
| ], | ||
| "blocked_live_reason": "", | ||
| "blocked_live_reason": "production_snapshot_artifact_route_not_configured", | ||
| "runtime_artifacts": { | ||
| "feature_snapshot": { | ||
| "required": true | ||
| } | ||
| }, | ||
| "features": { | ||
| "income_layer": false, | ||
| "option_overlay": false, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,18 @@ | |
| "ibkr": "IBKR_CASH_ONLY_EXECUTION", | ||
| "firstrade": "FIRSTRADE_CASH_ONLY_EXECUTION", | ||
| } | ||
| PLATFORM_FEATURE_SNAPSHOT_VARIABLES = { | ||
| "schwab": ("SCHWAB_FEATURE_SNAPSHOT_PATH", "SCHWAB_FEATURE_SNAPSHOT_MANIFEST_PATH"), | ||
| "longbridge": ( | ||
| "LONGBRIDGE_FEATURE_SNAPSHOT_PATH", | ||
| "LONGBRIDGE_FEATURE_SNAPSHOT_MANIFEST_PATH", | ||
| ), | ||
| "ibkr": ("IBKR_FEATURE_SNAPSHOT_PATH", "IBKR_FEATURE_SNAPSHOT_MANIFEST_PATH"), | ||
| "firstrade": ( | ||
| "FIRSTRADE_FEATURE_SNAPSHOT_PATH", | ||
| "FIRSTRADE_FEATURE_SNAPSHOT_MANIFEST_PATH", | ||
| ), | ||
| } | ||
| INCOME_LAYER_VARIABLES = ( | ||
| "INCOME_LAYER_ENABLED", | ||
| "INCOME_LAYER_START_USD", | ||
|
|
@@ -747,6 +759,57 @@ def _market_plan_for_strategy(strategy_profile: str) -> dict[str, str]: | |
| return market | ||
|
|
||
|
|
||
| def _feature_snapshot_extra_variables( | ||
| platform: str, | ||
| strategy_profile: str, | ||
| extra_variables: dict[str, Any], | ||
| ) -> dict[str, str]: | ||
| variable_names = PLATFORM_FEATURE_SNAPSHOT_VARIABLES.get(platform) | ||
| if not variable_names: | ||
| return {} | ||
| snapshot_variable, manifest_variable = variable_names | ||
|
|
||
| config = _load_platform_config() | ||
| strategies = config.get("strategies") | ||
| strategy = strategies.get(strategy_profile) if isinstance(strategies, dict) else None | ||
| if not isinstance(strategy, dict): | ||
| raise ValueError(f"strategy {strategy_profile!r} is missing from the runtime artifact catalog") | ||
| runtime_artifacts = strategy.get("runtime_artifacts") or {} | ||
| if not isinstance(runtime_artifacts, dict): | ||
| raise ValueError(f"strategy {strategy_profile!r} runtime_artifacts must be an object") | ||
| feature_snapshot = runtime_artifacts.get("feature_snapshot") or {} | ||
| if not isinstance(feature_snapshot, dict): | ||
| raise ValueError( | ||
| f"strategy {strategy_profile!r} runtime_artifacts.feature_snapshot must be an object" | ||
| ) | ||
|
|
||
| explicit = snapshot_variable in extra_variables or manifest_variable in extra_variables | ||
| if explicit: | ||
| snapshot_path = extra_variables.get(snapshot_variable) | ||
| manifest_path = extra_variables.get(manifest_variable) | ||
| else: | ||
| snapshot_path = feature_snapshot.get("path") | ||
| manifest_path = feature_snapshot.get("manifest_path") | ||
| snapshot_path = snapshot_path.strip() if isinstance(snapshot_path, str) else "" | ||
| manifest_path = manifest_path.strip() if isinstance(manifest_path, str) else "" | ||
| if explicit and not feature_snapshot and (snapshot_path or manifest_path): | ||
| raise ValueError( | ||
| f"strategy {strategy_profile!r} does not accept feature snapshot artifacts" | ||
| ) | ||
| if bool(snapshot_path) != bool(manifest_path): | ||
|
Comment on lines
+793
to
+799
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a snapshot strategy, explicit values such as Useful? React with 👍 / 👎. |
||
| raise ValueError( | ||
| f"strategy {strategy_profile!r} feature snapshot path and manifest path must be configured together" | ||
| ) | ||
| if feature_snapshot.get("required") is True and not (snapshot_path and manifest_path): | ||
| raise ValueError( | ||
| f"strategy {strategy_profile!r} requires feature snapshot path and manifest path" | ||
| ) | ||
| return { | ||
| snapshot_variable: snapshot_path, | ||
| manifest_variable: manifest_path, | ||
| } | ||
|
|
||
|
|
||
| def _build_runtime_target(args: argparse.Namespace) -> dict[str, Any]: | ||
| platform = _normalize_platform(args.platform) | ||
| target_name = _normalize_target_name(args.target_name) | ||
|
|
@@ -903,6 +966,13 @@ def build_switch_target(args: argparse.Namespace) -> dict[str, Any]: | |
| _reject_direct_dca_extra_variables(extra_variables) | ||
| _reject_direct_ibit_zscore_exit_extra_variables(extra_variables) | ||
| _reject_research_only_extra_variables(extra_variables) | ||
| extra_variables.update( | ||
| _feature_snapshot_extra_variables( | ||
| platform, | ||
| runtime_target["strategy_profile"], | ||
| extra_variables, | ||
| ) | ||
| ) | ||
|
|
||
| if args.set_platform_dry_run_variable: | ||
| extra_variables[PLATFORM_DRY_RUN_VARIABLES[platform]] = env_string(runtime_target["dry_run_only"]) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
service_targets_mode=offis used against an IBKR repository that has no existingCLOUD_RUN_SERVICE_TARGETS_JSON, the earlier bypass guard permits the operation, but this condition skips the deployment planner entirely andapply=trueproceeds directly to GitHub variable writes. This leaves the supported single-service/no-inventory path outside the new fail-closed contract and can publish live settings that the IBKR planner would reject; either synthesize a candidate inventory for this path or reject IBKR writes when no planner can run.Useful? React with 👍 / 👎.