Skip to content

feat(strategy): require real crypto performance evidence#125

Open
Pigbibi wants to merge 47 commits into
mainfrom
quant-strategy-s0-crypto
Open

feat(strategy): require real crypto performance evidence#125
Pigbibi wants to merge 47 commits into
mainfrom
quant-strategy-s0-crypto

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • remove fixed placeholder metrics from the production lifecycle adapter
  • require a prepared real market panel and fail closed with InsufficientEvidenceError
  • wire the production publish workflow to export, inject CRYPTO_LIFECYCLE_PREFLIGHT_ROOT through GITHUB_ENV, and invoke the public no-arg factory against the real artifact before upload
  • version the preflight manifest as crypto.lifecycle_preflight.v2; strictly validate v2 identity, counts, symbols, date ranges, and finite numeric content; retain strictly validated legacy v1 compatibility
  • preserve explicit panel injection for tests/research only; no live/order/deploy/runtime switch changes

Runtime rollout order

  1. Publish Lifecycle Inputs downloads real Binance history and exports the v2 bundle.
  2. The same job validates the bundle and writes CRYPTO_LIFECYCLE_PREFLIGHT_ROOT to GITHUB_ENV before calling build_backtest_runner() with no arguments.
  3. The real runner executes against the verified panel; missing, malformed, stale, non-finite, unknown-version, or mismatched artifacts raise InsufficientEvidenceError.
  4. Only after this preflight succeeds is the bundle uploaded for the downstream QPK reusable drift-check caller. Existing v1 bundles remain readable only after the same panel/schema/count/date/fixed-profile checks; regenerated v2 artifacts are the rollout target.

Validation

  • Crypto full: 57 passed
  • baseline review: insufficient_evidence, promotion_allowed=false
  • no live/order/deploy/runtime switch changes

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

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

🚫 Merge blocked: contract arbitration failed closed; automatic remediation is disabled

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in src/strategy_lifecycle/backtest_wrapper.py

CryptoBacktestRunner.run() only calls load_preflight_panel(start_date, end_date) the first time a no-arg runner is used. After _runner is cached, later calls with a wider or different date window reuse the old runner without revalidating start_date/end_date, so the new fail-closed evidence checks can be bypassed within the same process. (line 189)

Suggestion: For no-arg runners, revalidate the requested window on every run() call before executing, or avoid caching _runner across calls unless the cached panel is proven to satisfy the new request.

2. 🟠 [HIGH] Logic in src/strategy_lifecycle/backtest_wrapper.py

The new workflow computes an exact CRYPTO_LIFECYCLE_EXPECTED_END_DATE, but load_preflight_panel() only rejects bundles whose scored panel ends before that date. A bundle that extends past the requested completed day, including same-day partial data, still passes validation and will be treated as valid production evidence. (line 144)

Suggestion: When expected_end_date is provided, require the validated bundle to end exactly on that date (and apply the same exact-match rule to the market-history end date if that artifact is part of the contract).

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in src/strategy_lifecycle/backtest_wrapper.py

The v2 preflight validator no longer enforces per-symbol boundary coverage for market_history.csv.gz. It checks only that BTC/ETH overlap at least 99% with BTC's dates, so an artifact missing the first or last segment for ETH can still pass as long as the overlap stays above 99%. That weakens the strict date-range validation promised for the rollout. (line 103)

Suggestion: Mirror the exporter-side checks in the loader by also requiring each required market symbol to cover the full reference range (min(symbol_dates) <= min(reference_dates) and max(symbol_dates) >= max(reference_dates)), not just a 99% overlap ratio.


Review by Codex PR Review bot • PR

Pigbibi and others added 4 commits July 11, 2026 20:12
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: cb6c6e334e

ℹ️ 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/strategy_lifecycle/backtest_wrapper.py
Comment thread src/strategy_lifecycle/backtest_wrapper.py
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Pigbibi and others added 8 commits July 11, 2026 20:28
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: 6fdfb8fbba

ℹ️ 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 scripts/run_crypto_live_pool_baseline_review.py
Comment thread src/strategy_lifecycle/backtest_wrapper.py
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Pigbibi and others added 6 commits July 11, 2026 21:05
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: e9c9020311

ℹ️ 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 scripts/run_crypto_live_pool_baseline_review.py Outdated
Pigbibi and others added 3 commits July 11, 2026 21:44
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: 12a085da0f

ℹ️ 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/strategy_lifecycle/backtest_wrapper.py Outdated
Comment thread scripts/run_crypto_live_pool_baseline_review.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Pigbibi and others added 3 commits July 11, 2026 22:02
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Pigbibi and others added 2 commits July 11, 2026 22:25
Co-Authored-By: Codex <noreply@openai.com>
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: 97fbba3c64

ℹ️ 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/strategy_lifecycle/backtest_wrapper.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Comment thread scripts/run_crypto_live_pool_baseline_review.py Outdated
Comment thread scripts/run_crypto_live_pool_baseline_review.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py
Pigbibi and others added 2 commits July 11, 2026 22:39
Co-Authored-By: Codex <noreply@openai.com>
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: 4983135454

ℹ️ 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/strategy_lifecycle/backtest_wrapper.py
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Pigbibi and others added 3 commits July 11, 2026 23:47
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: ec24b82309

ℹ️ 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/strategy_lifecycle/backtest_wrapper.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py Outdated
Pigbibi and others added 2 commits July 12, 2026 02:12
Co-Authored-By: Codex <noreply@openai.com>
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: f2da506019

ℹ️ 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 scripts/export_lifecycle_preflight_inputs.py Outdated
Comment thread src/strategy_lifecycle/backtest_wrapper.py
Pigbibi and others added 10 commits July 12, 2026 02:25
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: 838cc0db03

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

strategy_index = header.index("strategy")
if not any(len(row) > strategy_index and row[strategy_index].strip().lower() == "final_score" for row in rows):
return False
return bool(set(header).intersection(metrics)) and _has_numeric_value(header, rows, metrics)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require metrics on the final_score row

When performance_summary.csv has a strategy column and includes a final_score row whose metric cells are blank/NaN, this still scans every row for a finite metric value. A numeric rule_score row therefore makes build_review() report BASELINE_REVIEW_NOT_YET_FROZEN even though the required final_score performance evidence is missing; the fresh evidence beyond the prior row-existence issue is that a placeholder final_score row is enough to pass. Restrict the numeric metric check to the final_score row when that column is present.

Useful? React with 👍 / 👎.

Comment on lines +153 to +154
datetime.fromisoformat(row[positions["test_end"]].replace("Z", "+00:00"))
except (IndexError, TypeError, ValueError):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject reversed walk-forward windows

When a hand-built or corrupted walkforward_validation_summary.csv has test_start after test_end, both timestamps still parse successfully and a finite metric makes _usable_walkforward_csv() return true. That moves the fail-closed review to BASELINE_REVIEW_NOT_YET_FROZEN even though the OOS window is chronologically invalid; require test_start <= test_end before accepting the row as evidence.

Useful? React with 👍 / 👎.

if any(key in manifest for key in ("market_start_date", "market_end_date")):
if manifest.get("market_start_date") != market_start or manifest.get("market_end_date") != market_end:
raise InsufficientEvidenceError("v1 market date range does not match manifest")
return panel.set_index(["date", "symbol"]).sort_index()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject duplicate panel keys before backtesting

If an env-supplied preflight bundle contains duplicate (date, symbol) rows and its manifest counts match that CSV, this validation returns the duplicated index as-is. The delegated backtest later pivots with unstack("symbol"), so the production lifecycle path raises a raw duplicate-index error instead of a controlled InsufficientEvidenceError; check for duplicate date/symbol keys before returning the panel.

Useful? React with 👍 / 👎.

Comment on lines +66 to +68
required = {"date", "symbol", "in_universe", "open", "final_score"}
if not required.issubset(panel.columns):
raise InsufficientEvidenceError("research_panel.csv.gz missing required columns")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject rows without a panel symbol

When an env-supplied bundle has a blank symbol on a scored in-universe row and the manifest row count is adjusted to match, this only checks that the column exists, then later compares manifest symbols after dropna(). The malformed row can therefore reach run_single_backtest(), where selecting it introduces a NaN symbol into the weight vector and can crash with a raw sort/indexing error instead of failing closed; validate that every panel symbol is present and non-empty before accepting the bundle.

Useful? React with 👍 / 👎.

Pigbibi and others added 3 commits July 12, 2026 07:32
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
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: 119a83d590

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

expected_end_date = date.fromisoformat(os.environ["CRYPTO_LIFECYCLE_EXPECTED_END_DATE"])
result = runner.run(
"crypto_live_pool_rotation",
{},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Verify the real strategy configuration

When the publish job reaches this verification step, it calls the lifecycle runner with an empty param set, so CryptoLivePoolBacktestRunner falls back to its hard-coded DEFAULT_BACKTEST_CONFIG (top_n=2, weighting='equal'). I checked the production research path: src/pipeline.py backtests using config/default.yaml, whose strategy is top_n: 3 and weighting: inverse_vol. This means the uploaded preflight can pass while validating a different portfolio than the crypto live-pool rotation operators actually run; pass the loaded strategy config or make the runner defaults match the live config.

Useful? React with 👍 / 👎.

raise InsufficientEvidenceError("market history preflight artifact is stale")
if expected_start_date is not None and panel_start_date > expected_start_date:
raise InsufficientEvidenceError("research panel starts after requested evaluation window")
if expected_end_date is not None and panel_end_date < expected_end_date:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require the requested end date to exist

When an env-supplied bundle skips the requested expected_end_date but has a later scored date, this coverage check still passes. The runner then slices to <= end_date, silently evaluates only the earlier available rows, and _metrics_to_qpk_result() reports the supplied end date, so a verification run for yesterday can publish evidence as if that completed date was present; require the requested end date to be in the scored panel dates before delegating.

Useful? React with 👍 / 👎.

if any(key in manifest for key in ("market_start_date", "market_end_date")):
if manifest.get("market_start_date") != market_start or manifest.get("market_end_date") != market_end:
raise InsufficientEvidenceError("v1 market date range does not match manifest")
return panel.set_index(["date", "symbol"]).sort_index()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Trim unscored warm-up rows before backtesting

When the exported bundle includes unscored warm-up rows before the manifest start, this returns the entire CSV to the runner. For no-start lifecycle runs such as the new publish verification, run_single_backtest() builds returns over all trading dates, so pre-score days are counted as zero-return observations and the result start date precedes the first scored evidence; trim to the scored date range or pass the scored start date into the runner.

Useful? React with 👍 / 👎.

"""Factory for the Crypto backtest runner."""
return CryptoBacktestRunner()
if self._panel is None:
self._runner = CryptoLivePoolBacktestRunner(panel=load_preflight_panel(start_date, end_date))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Export inverse-vol inputs for preflight runs

When the lifecycle runner is used with the production weighting: inverse_vol setting, this delegated preflight panel still only contains the exported lifecycle columns (in_universe, open, final_score). select_portfolio() applies inverse-vol weighting only when vol20 is present, otherwise it silently uses equal weights, so even after passing the real strategy params this adapter cannot reproduce the production backtest evidence unless the preflight bundle includes the weighting input.

Useful? React with 👍 / 👎.

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