feat(market-data): aggregate verified Polymarket evidence - #136
Conversation
📝 WalkthroughWalkthroughAdds ChangesVerified evidence set aggregation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VerifiedEvidence
participant aggregate_verified_polymarket_evidence
participant validate_members
participant VerifiedPolymarketEvidenceSet
VerifiedEvidence->>aggregate_verified_polymarket_evidence: submit verified members
aggregate_verified_polymarket_evidence->>validate_members: validate ordering and coverage
validate_members-->>aggregate_verified_polymarket_evidence: return aggregate bounds
aggregate_verified_polymarket_evidence->>VerifiedPolymarketEvidenceSet: create validated set
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 |
869bb21 to
fffdb5b
Compare
fffdb5b to
0d1ca71
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/set.rs (1)
74-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the promised empty-input rejection test.
The guard is correct, but no test exercises it despite empty-input validation being an explicit PR objective.
Proposed test
+ #[test] + fn rejects_empty_evidence_set() { + let error = aggregate_verified_polymarket_evidence(Vec::new()).unwrap_err(); + assert!(error.to_string().contains("must not be empty"), "{error:#}"); + }🤖 Prompt for 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. In `@rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/set.rs` around lines 74 - 77, Add a test for the verified evidence-set constructor or validation path that supplies an empty members collection and asserts it rejects the input with the “verified evidence set must not be empty” error. Keep existing valid-input tests unchanged and place the test alongside the relevant evidence-set tests in set.rs.
🤖 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/tasks/todo.md`:
- Line 41: Mark the implemented Polymarket evidence aggregation task as complete
in the todo tracker by changing its unchecked checkbox to checked. Keep the task
wording unchanged and limit the update to that tracker entry.
---
Nitpick comments:
In
`@rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/set.rs`:
- Around line 74-77: Add a test for the verified evidence-set constructor or
validation path that supplies an empty members collection and asserts it rejects
the input with the “verified evidence set must not be empty” error. Keep
existing valid-input tests unchanged and place the test alongside the relevant
evidence-set tests in set.rs.
🪄 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: a184b63f-555d-4009-a5be-78cccd6bf08f
📒 Files selected for processing (3)
rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/mod.rsrust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/set.rsrust_hft/prediction-markets/tasks/todo.md
Change contract\n\nAggregate independently sealed and verified Polymarket evidence handles into one read-only, contiguous BTC/SOL evidence set without accepting raw rows or weakening any member trust boundary.\n\n## Out of scope\n\nSnapshot construction, CLI acquisition, OSS selection/downloading, prediction audit generation, evaluator execution, collector changes, deployment, and live execution.\n\n## Dependency or merge order\n\nNone. This focused data-layer seam can merge independently before downstream multi-member snapshot and audit consumers.\n\n## Focused validation\n\n- Focused GitHub Linux run: https://github.com/proerror77/monday/actions/runs/29646163256\n- Rust 1.91.0 formatting completed with no further diff.\n- Evidence-set counterexamples passed for empty input, gaps, overlap, duplicate artifact digests, duplicate market/condition/token/trade identities, missing or duplicate BTC/SOL slots, and misaligned manifest ranges.\n- Positive coverage passed for two independently sealed and verified continuous 60-minute members.\n- Focused library clippy with warnings denied and package check passed.\n- Formal CI identified the mechanically required policy identity as sha256:7dd434d5401c17245313e6e1db78cd6dd9d7fde6033dafa8e1d1be544fdd2884; both BTC/SOL mission templates are pinned to that exact generated value in this PR.\n- Scope: 5 files, 474 non-generated changed lines.\n\n## Rollout/rollback impact\n\nLibrary-only and fail-closed; no runtime rollout. The mission-template edits only keep the checked-in governed policy identity synchronized with this source change. Roll back by reverting this PR before downstream consumers depend on the aggregate handle.