fix(polymarket): bind evidence to explicit episodes - #183
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughPolymarket evidence selection now requires explicit market IDs. Discovery, normalization, publication, manifest sealing, and verification propagate and enforce those IDs through updated v2/v3 schemas, dynamic symbol lists, and fail-closed validation. ChangesExplicit market selection and evidence binding
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CLI as PublishPolymarketEvidence
participant Selection as ResearchSelection
participant Normalizer as normalize_raw
participant Publisher as publish_normalized
participant Verifier as verify_polymarket_evidence
CLI->>Selection: provide market_ids
Selection->>Selection: validate and filter requested markets
Selection-->>Normalizer: return selected contracts
Normalizer->>Normalizer: derive market_ids and symbols
Normalizer-->>Publisher: provide normalized_evidence.v2
Publisher->>Publisher: emit evidence_artifact.v3 manifest
Publisher-->>Verifier: provide manifest and evidence triplet
Verifier->>Verifier: enforce explicit market selection
Verifier-->>CLI: accept or reject evidence
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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.
🧹 Nitpick comments (1)
rust_hft/tools/collector/src/polymarket_evidence_artifact.rs (1)
148-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded symbol allow-list duplicates the existing
SYMBOLSconstant in the same crate.
["BTCUSDT", "SOLUSDT"]is inlined here, butpolymarket_research_select.rs(same crate,rust_hft/tools/collector) already defines/uses aSYMBOLSconstant for the identical allow-list. If the supported symbol set is ever extended, this literal could silently drift from the canonical list and either wrongly reject valid evidence or (if under-restricted) accept an unintended symbol.♻️ Suggested fix: reuse the shared constant
- || report - .symbols - .iter() - .any(|symbol| !["BTCUSDT", "SOLUSDT"].contains(&symbol.as_str())) + || report + .symbols + .iter() + .any(|symbol| !crate::polymarket_research_select::SYMBOLS.contains(&symbol.as_str()))#!/bin/bash # Confirm SYMBOLS visibility in polymarket_research_select.rs to validate reuse is feasible rg -n 'const SYMBOLS' rust_hft/tools/collector/src/polymarket_research_select.rs🤖 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/tools/collector/src/polymarket_evidence_artifact.rs` around lines 148 - 152, Replace the inline symbol allow-list in the report validation condition with the existing shared SYMBOLS constant from polymarket_research_select, preserving the current empty-symbol and unsupported-symbol checks. Ensure the constant is accessible from polymarket_evidence_artifact and use it as the sole source of supported symbols.
🤖 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.
Nitpick comments:
In `@rust_hft/tools/collector/src/polymarket_evidence_artifact.rs`:
- Around line 148-152: Replace the inline symbol allow-list in the report
validation condition with the existing shared SYMBOLS constant from
polymarket_research_select, preserving the current empty-symbol and
unsupported-symbol checks. Ensure the constant is accessible from
polymarket_evidence_artifact and use it as the sole source of supported symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4b4a7fe-8770-45a9-b1be-6885202728e6
📒 Files selected for processing (6)
rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rsrust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/verified.rsrust_hft/tools/collector/src/bin/polymarket-raw-ops.rsrust_hft/tools/collector/src/polymarket_evidence_artifact.rsrust_hft/tools/collector/src/polymarket_research_normalize.rsrust_hft/tools/collector/src/polymarket_research_select.rs
43f4d47 to
9847c88
Compare
Change contract
Require every published Polymarket evidence artifact to name its exact non-empty market-ID episode set, and fail closed unless producer selection, v3 manifest, sealed rows, typed verification, and the checked-in Polymarket mission policy pins agree.
Acceptance evidence
cargo test --manifest-path rust_hft/prediction-markets/Cargo.toml -p ploy-market-data— 45 passed.cargo test --manifest-path rust_hft/tools/collector/Cargo.toml— 185 passed, 2 existing ignored.seal -> verify_polymarket_evidenceretains both independent Up/Down token books and rejects an order-book row for an unrequested market.git diff --checkpasses. Linux-only publication tests remain for GitHub CI.Out of scope
No Binance LOB behavior, collector deployment/cutover, cloud mutation, data publication, snapshot construction/evaluator/MCTS, or execution policy behavior change.
Dependency / merge order
Base:
main. Implements issue #182 under PRD issue #181, and unblocks the separate #173 evidence-compile rerun after merge and image publication.Rollout / rollback impact
No runtime rollout. New compiler outputs use manifest v3 and require explicit
--market-id; the verifier retains v2 compatibility for already-published artifacts. Rollback is a source/image rollback with no data overwrite.