research: build snapshots from verified market artifacts - #133
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds a verified-artifact Verified snapshot pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VerifiedBinanceMarketTape
participant VerifiedPolymarketEvidenceSet
participant build_research_snapshot_from_verified_artifacts
participant ResearchSnapshotManifest
participant ResearchSnapshot
VerifiedBinanceMarketTape->>build_research_snapshot_from_verified_artifacts: provide verified Binance tape
VerifiedPolymarketEvidenceSet->>build_research_snapshot_from_verified_artifacts: provide verified Polymarket evidence
build_research_snapshot_from_verified_artifacts->>ResearchSnapshotManifest: validate artifacts and construct audit metadata
build_research_snapshot_from_verified_artifacts->>ResearchSnapshot: build updates, outcomes, factors, and book snapshots
ResearchSnapshotManifest-->>ResearchSnapshot: attach manifest and row counts
Possibly related PRs
🚥 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rs`:
- Around line 539-548: In the EventDiscovered update construction, set
resolved_up_won to None so pre-settlement replay cannot expose the official
outcome. Preserve label attachment through bind_and_filter_verified_observations
after settlement.available_at.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6dd64a93-df8a-45cc-b363-c0e6d97b107d
⛔ Files ignored due to path filters (1)
rust_hft/prediction-markets/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
rust_hft/prediction-markets/crates/ploy-research/Cargo.tomlrust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.sha256rust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.txtrust_hft/prediction-markets/crates/ploy-research/src/lib.rsrust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rsrust_hft/prediction-markets/tasks/todo.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 810341b1cb
ℹ️ 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".
810341b to
951516b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rs`:
- Around line 193-198: Update the option validation containing the positive
cadence checks to also require options.pm_book_sample_secs not exceed
options.max_quote_age_secs. Preserve the existing validation error behavior and
reject invalid manifests during builder validation before snapshot creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12dcbf6b-a569-4702-bf6f-e29062b258b2
📒 Files selected for processing (6)
rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.jsonrust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.jsonrust_hft/prediction-markets/crates/ploy-research/src/factors.rsrust_hft/prediction-markets/crates/ploy-research/src/lib.rsrust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rsrust_hft/prediction-markets/tasks/todo.md
🚧 Files skipped from review as they are similar to previous changes (1)
- rust_hft/prediction-markets/crates/ploy-research/src/lib.rs
Change contract
Build one deterministic ResearchSnapshot from externally anchored
VerifiedBinanceMarketTapeandVerifiedPolymarketEvidenceSethandles, deriving and validating the canonical artifact audit internally before any research projection or label binding.Out of scope
Dependency or merge order
Base: main after #140 and #141. Merge this adapter before the focused verified-artifact CLI and cloud E2E layer. No stacked base PR remains.
Focused validation
git diff --checkis clean and no newrustfmt::skipwas added.sha256:c7ccfd3bd8fc0ab7cd1444768cfe8b53b95baefc8973b47f34d5156b31d234c0.Rollout / rollback impact
Research-only opt-in API. Existing database snapshot compilation and live execution remain unchanged. Rollback is a single PR revert before the dependent CLI layer.