fix(market-data): accept ordered same-hour references - #213
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5c5e12085
ℹ️ 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".
| let ordered_same_hour = next_hour == current_hour | ||
| && parse_time(&pair[1].start_recorded_at, "start_recorded_at")? | ||
| >= parse_time(&pair[0].end_recorded_at, "end_recorded_at")?; |
There was a problem hiding this comment.
Reject duplicate point-in-time reference fragments
When a valid same-hour segment has start_recorded_at == end_recorded_at, listing that segment twice makes the second start equal the first end, so this >= check accepts two identical fragments. This violates the non-overlap contract and can double-count or duplicate reference provenance; aggregate_reference_trade_completions will not catch it when the fragment has no completion entries. Reject repeated segment digests/identities, while still allowing distinct fragments that share a timestamp.
Useful? React with 👍 / 👎.
|
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 (3)
📝 WalkthroughWalkthroughThe PR updates search policy snapshot identifiers for Polymarket BTC and SOL 5-minute missions and changes V2 reference-fragment validation to accept ordered, non-overlapping fragments within the same UTC hour, with tests covering accepted and rejected orderings. ChangesReference fragment validation
Mission snapshot configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.jsonTraceback (most recent call last): rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.jsonTraceback (most recent call last): 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 |
Change contract
Allow validated Polymarket evidence to bind multiple chronologically ordered, non-overlapping immutable reference fragments from the same UTC hour, while preserving fail-closed rejection of reversed, overlapping, or skipped-hour sequences.
Out of scope
Collector/importer changes, evidence byte rewrites, #210/#211 timestamp rules, ResearchSnapshot/cohort construction, factor/label changes, evaluator/MCTS, Paper/Live, 15m/1h.
Dependency / merge order
Base:
main@344a9bf39a4d2036ef12335af4671b8d12b290d7. No stacked code dependency. Relates to #212; keep #212 open until the fresh exact-main ACK verifier advances through this gate. This focused fix blocks cloud completion of #173 and therefore #169.Focused validation
polymarket-evidence-typed-verify-173-0722xon exact-main imageresearch-runner@sha256:6057938c3fdde14b3b7d9df467a8cd897b068b06ca96807c4b668c9fd2497cc9rejects authenticated reference hours10,11,11,11withvalidated reference inputs must be consecutive UTC hours.seals_ordered_same_hour_reference_fragmentsfailed before the predicate change.cargo test -p ploy-market-data— 50 passed.cargo fmt -p ploy-market-data -- --check— passed.cargo clippy -p ploy-market-data --all-targets --no-deps -- -D warnings— passed.Rollout / rollback impact
Rollout publishes a new immutable
research-runnerimage, then reruns the existing read-only ACK verifier against the exact #173 evidence digests. Rollback is a one-commit revert; no data migration, raw rewrite, or collector change.Why no PRD
This is a small, already-specified verifier defect with one sequence predicate and targeted counterexamples, so Matt TDD is the narrowest workflow.
Summary by CodeRabbit
Bug Fixes
Tests