Skip to content

fix: drive strategy schedules from settings catalog#192

Merged
Pigbibi merged 6 commits into
mainfrom
fix/config-driven-strategy-schedules
Jul 20, 2026
Merged

fix: drive strategy schedules from settings catalog#192
Pigbibi merged 6 commits into
mainfrom
fix/config-driven-strategy-schedules

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move market and strategy scheduler profiles into platform-config.json
  • resolve schedules through domain defaults, strategy overrides, and enabled-plugin overrides
  • validate timezone, cron shape, and all scheduler profile references

Why

IBKR accounts can switch strategies whose markets and execution windows differ. Scheduler selection must follow the runtime strategy catalog instead of Python strategy-name prefixes and hardcoded mappings.

Validation

  • python/scripts/build_config.py --check
  • 68 tests + 16 subtests passed in python/tests/test_runtime_settings.py
  • Ruff passed on changed Python files
  • all 26 catalog strategies resolve; IBIT plugin override resolves to daily schedule

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

Merge allowed: 3 finding(s) reported but none are blocking

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in python/scripts/build_config.py

Timezone validation now depends on the host OS timezone database via ZoneInfo(...). On minimal CI/runtime images that do not have tzdata installed, valid IANA zones already present in platform-config.json such as America/New_York and Asia/Hong_Kong will be reported as invalid, so the documented python/scripts/build_config.py --check entry point starts failing for correct configs purely because of the container image. (line 64)

Suggestion: Make timezone validation independent of the host image by declaring/installing tzdata for this toolchain, or validate against a bundled/controlled IANA zone source instead of relying on whatever the OS happens to provide.

2. 🟡 [MEDIUM] Logic in python/scripts/build_config.py

The new cron validation only checks that each scheduler field has 2 or 5 whitespace-separated tokens. Expressions like 61 24 * * *, foo bar * * *, or 99 nope will pass --check even though they are not runnable schedules. That means a bad profile can be accepted into platform-config.json and only fail later when a runtime target is built or, worse, execute at an unintended time. (line 69)

Suggestion: Validate scheduler expressions with the same parser/range checks used by the runtime target validator so build_config.py --check rejects invalid cron/time values instead of only counting tokens.

3. 🟡 [MEDIUM] Reliability in python/scripts/build_config.py

validate() now iterates config.get("domains", {}).items() without first confirming that domains is actually an object. If platform-config.json contains a malformed top-level domains value such as a list or string, the validator throws AttributeError instead of returning a normal validation error list. Because build_config.py --check is the advertised guardrail for this file, that turns a bad config edit into a crash rather than a diagnosable validation failure. (line 82)

Suggestion: Guard the top-level domains value with an isinstance(..., dict) check before iterating and append a validation error when it is not an object.


Review by Codex PR Review bot • PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8f0616a18

ℹ️ 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".

Comment thread python/scripts/build_runtime_switch.py
Comment thread python/scripts/build_config.py
Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d66d14a31b

ℹ️ 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".

Comment thread python/scripts/build_config.py Outdated
Pigbibi and others added 2 commits July 21, 2026 07:06
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi enabled auto-merge (squash) July 20, 2026 23:11
Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a496a7b97

ℹ️ 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".

Comment thread python/scripts/build_config.py Outdated
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi merged commit b8a5d0b into main Jul 20, 2026
10 of 11 checks passed
@Pigbibi
Pigbibi deleted the fix/config-driven-strategy-schedules branch July 20, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant