Skip to content

feat(strategy): add fail-closed strategy review contract - #246

Merged
Pigbibi merged 30 commits into
mainfrom
quant-strategy-s0-qpk
Jul 11, 2026
Merged

feat(strategy): add fail-closed strategy review contract#246
Pigbibi merged 30 commits into
mainfrom
quant-strategy-s0-qpk

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add strategy_review.v1 schema and fail-closed validator
  • export source revision, cost model, and data timestamp metadata
  • add negative tests for operational/placeholder/failed-gate evidence

Validation

  • QPK full: 571 passed, 1 skipped
  • targeted: 11 passed
  • no promotion/live/runtime changes

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

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

ℹ️ Other Findings

1. 🟡 [MEDIUM] Logic in scripts/validate_strategy_review.py

A top-level decision of fail is currently accepted even when all 12 hard gates are pass and blocking_reason_codes is empty. The validator only checks consistency for the pass path, so a producer can publish a self-contradictory packet whose gates and score say the strategy passed but whose final decision hard-rejects it. That will produce incorrect review outcomes and poison downstream audit/history data. (line 237)

Suggestion: Reject any non-pass decision unless at least one hard gate is fail/insufficient_evidence or explicit blocking reasons are present, and add a regression test for an all-pass / decision="fail" payload.

2. 🟡 [MEDIUM] Reliability in scripts/validate_strategy_review.py

The validator only requires evidence_refs when a hard gate is marked pass. A gate with status="fail" or status="insufficient_evidence" can still validate with empty evidence_refs (and even empty reason_codes), which means a strategy can be blocked without any auditable artifact trail explaining why the gate failed. That undercuts the stated fail-closed review contract and makes downstream review/debugging unreliable. (line 225)

Suggestion: Require non-empty reason_codes and evidence_refs for every hard gate whose status is not pass, and add a negative test covering a failed gate with no supporting evidence.


Review by Codex PR Review bot • PR

Pigbibi and others added 29 commits July 11, 2026 23:27
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>
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>
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>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi force-pushed the quant-strategy-s0-qpk branch from 7197b60 to 6aa2a8c Compare July 11, 2026 15:27
@Pigbibi
Pigbibi merged commit ff09c88 into main Jul 11, 2026
3 checks passed
@Pigbibi
Pigbibi deleted the quant-strategy-s0-qpk branch July 11, 2026 15:33
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