Skip to content

feat: add SOXL volatility scaling research overlay#287

Closed
Pigbibi wants to merge 1 commit into
mainfrom
codex/t4s3-soxl-volatility-scaling-overlay-v1-20260723
Closed

feat: add SOXL volatility scaling research overlay#287
Pigbibi wants to merge 1 commit into
mainfrom
codex/t4s3-soxl-volatility-scaling-overlay-v1-20260723

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add the frozen lagged 20-session relative-volatility research overlay
  • preserve SMA200 next-open baseline and set-once provenance publication
  • add focused synthetic regression coverage

Validation

  • PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=src ... pytest -q tests/test_soxl_core_optimization.py tests/test_soxl_soxx_offline_input_contract.py tests/test_soxl_soxx_typed_baseline_result.py (18 passed)
  • compileall passed
  • Ruff unavailable; no dependency installation performed

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: 142d6473e3

ℹ️ 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 src/us_equity_strategies/research/soxl_core_optimization.py
@github-actions

Copy link
Copy Markdown

🤖 Codex PR Review

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

ℹ️ Other Findings

1. 🟡 [MEDIUM] Logic in src/us_equity_strategies/research/soxl_core_optimization.py

_volatility_window_metrics() starts peak at start_equity but leaves peak_index at -1. If the first bar in the window is already below start_equity, the first max-drawdown event stores max_drawdown_peak_index = -1, so the later recovery check can never succeed and max_drawdown_unrecovered_sessions is reported even when the equity curve fully recovers later in the same window. This is directly reachable because every run_soxl_volatility_scaling() call publishes these metrics in validation_metrics_c2_5 and post_lock_metrics. (line 454)

Suggestion: Represent the window-start peak explicitly when seeding the loop. For example, initialize a real peak anchor for start_equity and compare recovery against that peak value instead of relying on selected[max_drawdown_peak_index] with a sentinel -1 index.

2. 🔵 [LOW] Logic in src/us_equity_strategies/research/soxl_core_optimization.py

The new max_drawdown_recovery_sessions and max_drawdown_unrecovered_sessions fields are counted from max_drawdown_peak_index (the peak before the drawdown), not from the trough where the maximum drawdown was actually observed. On any multi-session selloff, this overstates the reported recovery duration by including the entire descent into the drawdown, so the persisted research metrics are numerically wrong. (line 468)

Suggestion: Track the index where max_drawdown is set (the trough), and compute both recovery and unrecovered session counts from that trough index rather than from the prior peak index.


Review by Codex PR Review bot • PR

@Pigbibi Pigbibi closed this Jul 23, 2026
Pigbibi added a commit that referenced this pull request Jul 23, 2026
Publication-only replacement for closed PR #287 after a PR-object/ref-binding anomaly.
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