Skip to content

fix: unify RSI2 SOXX acceptance benchmark period#291

Merged
Pigbibi merged 1 commit into
mainfrom
codex/t4s5-soxl-prior-close-unified-benchmark-recovery-20260723
Jul 23, 2026
Merged

fix: unify RSI2 SOXX acceptance benchmark period#291
Pigbibi merged 1 commit into
mainfrom
codex/t4s5-soxl-prior-close-unified-benchmark-recovery-20260723

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Implements the authorized T4/S5 prior-close unified SOXX benchmark-period recovery with tests-first evidence.

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

github-actions Bot commented Jul 23, 2026

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

selection_windows_all_active is now derived from winner is not None, but _select_rsi2_mean_reversion_winner() returns "UNSCALED_SMA200" whenever the baseline is active even if every non-baseline RSI2 candidate was skipped for zero activity. In that common path, the emitted gate and acceptance_classes.activity.selection_windows_all_active become true even though no RSI2 candidate actually had activity in all three selection windows, so the result JSON misreports the acceptance evidence for the current production entry point. (line 1046)

Suggestion: Compute this flag from the selected candidate's per-window activity_observed values, and keep it false unless a non-baseline candidate exists and is active in all three validation windows.

2. 🟡 [MEDIUM] Reliability in src/us_equity_strategies/research/soxl_core_optimization.py

soxx_drawdown_comparison.final_holdout_max_drawdown now comes from final_c2["soxx_close_path_max_drawdown"], so it becomes null whenever locked_winner is None. The SOXX final-holdout drawdown does not depend on picking a winner, and the function previously always reported it. This means the JSON result emitted by run_soxl_rsi2_mean_reversion loses the benchmark drawdown exactly on the new fail-closed/no-activity path, which breaks auditing and downstream consumers that expect benchmark evidence even for NO_IMPROVEMENT outcomes. (line 1072)

Suggestion: Always compute the final-holdout SOXX drawdown directly from soxx (or from the baseline metrics) and populate soxx_drawdown_comparison.final_holdout_max_drawdown regardless of whether a winner was selected.


Review by Codex PR Review bot • PR

Pigbibi commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Goal Driver exact-head arbitration for 707ccb9c10176bdda0073291249001da9868c96b: this finding is advisory, so no code change or remediation is authorized. The frozen T4/S5 acceptance contract explicitly requires locked_winner=null when no validation candidate qualifies while retaining the v1 schema. A fresh repository search found no current production reader that indexes post_lock_metrics[locked_winner] or validates a non-null winner; the current runner guards None, keeps baseline-only post-lock metrics, and persistence/readback perform a generic strict dict round-trip. Therefore the required blocking criteria (concrete current caller or declared public-untrusted boundary plus proven correctness/data-integrity impact) are not met. One failed-job rerun is being used solely for required-check settlement; remediation remains 0/1.

@Pigbibi
Pigbibi merged commit 0a76380 into main Jul 23, 2026
3 of 4 checks passed
@Pigbibi
Pigbibi deleted the codex/t4s5-soxl-prior-close-unified-benchmark-recovery-20260723 branch July 23, 2026 22: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