fix(research): preserve real BTC mission rejection evidence - #91
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR separates book imbalance features, enriches PIT provenance evidence, pins the alpha mission to a BTCUSDT MCTS configuration, and adds a report for a reproducible real-data E2E run that failed acceptance after walk-forward evaluation. ChangesBTCUSDT research flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Materializer
participant PITRows
participant Mission
participant E2EReport
Materializer->>PITRows: publish provenance and imbalance features
PITRows->>Mission: provide point-in-time feature data
Mission->>E2EReport: record walk-forward results and evidence
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2c0360081
ℹ️ 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".
|
|
||
| | Evidence | Value | | ||
| | --- | --- | | ||
| | Repository source revision | `6e3607ec11ed5470eb265480c06b0964f0653b21` | |
There was a problem hiding this comment.
This report is meant to be immutable run evidence, but the recorded source revision is not present in this repository (git cat-file -t 6e3607ec11ed5470eb265480c06b0964f0653b21 returns a bad object in this checkout). Anyone trying to audit or reproduce the run cannot check out the code that supposedly produced the evidence, so the run should be tied to a reachable commit or the report should clearly state where that revision is available.
Useful? React with 👍 / 👎.
| --materialization-url pit/541c1db8d839477a4ae530d96374f4ffe7a05a66c6bafbb619e8613ea38de760.materialization.json \ | ||
| --materialization-sha256 541c1db8d839477a4ae530d96374f4ffe7a05a66c6bafbb619e8613ea38de760 \ |
There was a problem hiding this comment.
Following the materialization command immediately above will not recreate this fixed 541c... report path/SHA, because the materializer hashes a report that includes its current created_at timestamp. In a fresh reproduction this file will be absent or the --materialization-sha256 check will fail unless the reader already has the original report, so the instructions should use the newly emitted report path/SHA or explain that the published materialization object must be downloaded instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2c0360081
ℹ️ 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".
| collector_manifest_path: PathBuf, | ||
| collector_manifest_sha256: String, | ||
| success_marker_path: PathBuf, | ||
| success_marker_sha256: String, |
There was a problem hiding this comment.
When a mission is supplied a binance-lob-pit-v1 materialization manifest that was produced before these fields existed, alpha-harness still deserializes each SourceSegment with only sha256 and validate_materialization only normalizes that one field, so ACK/local mission execution can accept evidence that lacks the collector manifest and _SUCCESS bindings this change is meant to make mandatory. Please either bump/version the materialization schema or update the mission-side validator to require these sidecar hashes before the run can claim raw manifest/_SUCCESS binding.
Useful? React with 👍 / 👎.
| --materialization-url pit/541c1db8d839477a4ae530d96374f4ffe7a05a66c6bafbb619e8613ea38de760.materialization.json \ | ||
| --materialization-sha256 541c1db8d839477a4ae530d96374f4ffe7a05a66c6bafbb619e8613ea38de760 \ |
There was a problem hiding this comment.
In this reproduction flow, the materialization command immediately above will not create pit/541c...materialization.json on a fresh run: lob-pit-materializer includes created_at = Utc::now() in the report before hashing it, and alpha-harness rejects any --materialization-sha256 mismatch. Unless the original published materialization JSON is also downloaded, following these commands against the listed raw segments fails before evaluation instead of reproducing the recorded mission.
Useful? React with 👍 / 👎.
Change contract
Make the pinned BTCUSDT top-of-book MCTS mission executable against byte-bound real LOB inputs and record the immutable fail-closed rejection produced by that exact run.
Acceptance evidence
book_imbalancefield while retaining the governed top-depth feature._SUCCESSbytes by SHA-256.6e3607ec, the real OSS inputs and result bundle, the rejected walk-forward metrics, unopened sealed holdout, and absent Paper/Shadow/Live handoff.Out of scope
Dependency or merge order
None. This is based on
mainafter #84 and #88 and preserves both changes.Focused validation
cargo fmt --manifest-path rust_hft/Cargo.toml -p hft-collector -- --checkcargo test --manifest-path rust_hft/Cargo.toml -p hft-collector --bin lob-pit-materializer— 10 passed.cargo test --manifest-path rust_hft/Cargo.toml -p alpha-harness parses_explicit_taker_execution_costs— 1 passed.cargo clippy --manifest-path rust_hft/Cargo.toml -p hft-collector --bin lob-pit-materializer -- -D warningsgit diff --checkpassed.Rollout/rollback impact
Research-only. Merge updates the materializer evidence schema and the example mission baseline; it does not deploy a runtime, use exchange credentials, or open execution. Revert this PR to restore the previous materializer/template/report together.
Summary by CodeRabbit
New Features
Documentation
Configuration