Skip to content

feat(backtest): default crypto research wrapper to orchestrator#75

Merged
Pigbibi merged 5 commits into
mainfrom
feat/crypto-equity-combo-orchestrator
Jul 10, 2026
Merged

feat(backtest): default crypto research wrapper to orchestrator#75
Pigbibi merged 5 commits into
mainfrom
feat/crypto-equity-combo-orchestrator

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary\n- Default crypto research wrapper to orchestrator/profile\n- Keep legacy path only for compatibility\n\n## Test Plan\n- python3 -m py_compile on modified script\n- --help smoke test

Pigbibi and others added 2 commits July 10, 2026 03:36
Wire CryptoEquityComboBacktestRunner, combo simulator, walk-forward CLI,
and research script --orchestrator path following HK equity combo pattern.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-Authored-By: Codex <noreply@openai.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

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

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

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

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review


P1 Badge Trim lookback rows out of fold metrics

When start_date is supplied by BacktestOrchestrator.walk_forward, _slice_history prepends min_history_days + 5 days as indicator lookback, but this line feeds that entire slice into run_combo_backtest and then returns result.metrics unchanged. The reported start_date is reset to the requested fold start, so CAGR, drawdown, total return, and Trading Days for combo walk-forward folds include pre-window returns, corrupting the fold results. Please keep the lookback for warm-up but compute/report metrics only on returns dated >= start_date.


return equity.pct_change().fillna(0.0)

P1 Badge Remove DCA cash deposits from reported returns

For any combo backtest, portfolio_values grows from the new dca_amount_usd contribution added each day, and this line converts that externally contributed cash into investment returns for _performance_metrics. Even with flat BTC/ETH prices this reports positive CAGR/Sharpe from deposits rather than market performance, so orchestrator results are materially overstated; compute returns on capital excluding cash flows or report a money-weighted metric explicitly.


combo_config=CryptoComboBacktestConfig(
combo_mode=cast(ComboMode, combo_mode),
min_history_days=min_history_days,
),

P2 Badge Honor combo allocation params in runner

When callers pass research params such as btc_weight, trend_weight, dca_amount_usd, or dynamic_trend_cut through the orchestrator, those values are recorded in the BacktestResult params but are not applied here because the config is built only from combo_mode and min_history_days. This makes parameter sweeps or single runs appear to test different allocations while every result still uses the dataclass defaults.


if len(close) < int(combo.min_history_days):
raise ValueError(
f"market_history requires at least {int(combo.min_history_days)} overlapping trading days"
)

P2 Badge Validate overlapping BTC/ETH history before reporting metrics

This check counts dates in the pivoted frame, not dates where both required assets have usable closes. If ETH is missing, all-NaN, or non-overlapping for an otherwise long BTC history, the check passes, _combo_daily_returns returns an empty series, and the backtest reports all-zero metrics instead of rejecting invalid input; validate the overlapping non-null BTC/ETH index here.

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

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

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@Pigbibi Pigbibi merged commit f4ffecf into main Jul 10, 2026
3 checks passed
@Pigbibi Pigbibi deleted the feat/crypto-equity-combo-orchestrator branch July 10, 2026 00:59
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