feat: add strict strategy evidence v2 promotion gate - #290
Conversation
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a3c84daf8
ℹ️ 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".
| "cost_model", | ||
| "risk_report", | ||
| "kelly_readiness_report", | ||
| from quant_platform_kit.strategy_lifecycle.evidence_package_v2 import ( |
There was a problem hiding this comment.
When this script is invoked directly from a source checkout without an editable install, this import runs before src is on sys.path, so python scripts/validate_strategy_evidence_package.py ... now exits with ModuleNotFoundError instead of validating anything. The sibling source-checkout wrapper adds ROOT / "src" before importing package code, and this script needs the same bootstrap or another standalone path to preserve the documented/tested direct CLI workflow.
Useful? React with 👍 / 👎.
| for field in _RESULT_NUMBER_FIELDS: | ||
| if field not in result: | ||
| continue | ||
| if field in {"observation_count", "param_version"}: | ||
| _finite_integer(result.get(field), f"{label}.{field}", issues, minimum=0) |
There was a problem hiding this comment.
If a v2 evidence package includes a tampered backtest.promotion_run.*.param_version, the dependency-free validator currently accepts values such as a string or 0: param_version is allowed in _RESULT_FIELDS, but it is absent from _RESULT_NUMBER_FIELDS, so this branch never visits it. That lets packages pass the Python/CLI gate even though the packaged schema requires param_version to be an integer with minimum 1.
Useful? React with 👍 / 👎.
| if ( | ||
| root is None | ||
| or not isinstance(raw_path, str) | ||
| or not _valid_artifact_path(raw_path) | ||
| ): | ||
| continue |
There was a problem hiding this comment.
When a v2 payload is validated through the raw-mapping APIs without base_dir (for example validate_evidence_package(payload) or validate_evidence_package_v2(payload)), root is None takes this continue path for every artifact, so the gate can return valid and even promotion_eligible without checking that the declared artifact files exist or that their bytes match the SHA-256 values. Since v2 promotion evidence is supposed to bind actual artifact bytes, the validator should fail closed or avoid promotion eligibility when it cannot perform those file checks.
Useful? React with 👍 / 👎.
| "observation_count", | ||
| "run_duration_seconds", | ||
| } | ||
| _closed_object(result, label, required, _RESULT_FIELDS, issues) |
There was a problem hiding this comment.
Because params and param_set_id are only allowed fields and are not part of the required result set, a v2 package can strip them from every backtest.promotion_run result and still pass validation. Those fields are emitted by BacktestResult.to_dict() and are the only structured binding between the promotion run metrics and the candidate parameter set, so accepting packages without them undermines the exact BacktestOrchestrator evidence identity check.
Useful? React with 👍 / 👎.
1 participant
Summary
Safety boundary
Validation