feat(polymarket): normalize five research surfaces - #95
proerror77 wants to merge 1 commit into
Conversation
|
@codex review |
|
Warning Review limit reached
Next review available in: 10 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 (2)
✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a24c17419
ℹ️ 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".
| } else { | ||
| bail!("line {}: trade token has no semantic outcome", pending.line) | ||
| }; | ||
| let trade_ts = required(&pending.update, "trade_ts")?.to_owned(); |
There was a problem hiding this comment.
When a selected market has trades outside its 5-minute event window, this loop still emits them because it never compares trade_ts/trade_ts_unix with contract.event_start/contract.event_end. The raw collector only bounds fetched trades by market_lookback_secs, and the existing rust_hft/tools/collector/src/polymarket_parity.rs::trade_map path filters trade_ts_unix to the event interval, so this normalization can contaminate research snapshots with pre/post-window trades while quotes and Chainlink references are window-filtered.
Useful? React with 👍 / 👎.
| fn settlement_fingerprint(pending: &Pending) -> Result<String> { | ||
| let canonical = json!({ | ||
| "sequence": pending.sequence, | ||
| "recorded_at": pending.recorded_at, | ||
| "update": pending.update | ||
| }); |
There was a problem hiding this comment.
If the same official settlement for a selected market is recorded twice with a different tape sequence or recorded_at, this fingerprint changes even though the settlement update payload is identical, causing record_settlement_fingerprint to reject it as a changed settlement. Since the check is meant to detect a changed settlement payload, including per-record envelope fields makes harmless duplicate rows fail normalization instead of deduping to one settlement_pair.
Useful? React with 👍 / 👎.
| let winner = required(&pending.update, "winning_token_id")?.to_owned(); | ||
| let fingerprint = settlement_fingerprint(&pending)?; | ||
| if record_settlement_fingerprint(settled, market_id, &fingerprint, pending.line)? { | ||
| let prices = json_strings(market.get("outcomePrices"), "outcomePrices")?; |
There was a problem hiding this comment.
When a valid settlement's raw market.outcomePrices is already a JSON array rather than a string containing an array, validate_market_settlement accepts it via parse_json_array, but this normalizer immediately reparses it with json_strings, which only accepts string values. That means an input segment can pass the governed validation step and then fail normalization, even though the settlement prices are valid and could be emitted.
Useful? React with 👍 / 👎.
|
Closing after the repository-mandated two-axis review found an architecture boundary violation: this collector layer must publish neutral typed staging evidence, not an evaluator-authoritative snapshot or raw venue wire payloads. The replacement PR will remove snapshot/label authority and raw payload leakage; canonical labels and snapshot_contract_hash remain owned by rust_hft/prediction-markets. |
Change contract
Normalize already validated and selected governed BTC/SOL five-minute Polymarket contracts into deterministic point-in-time research rows for five surfaces: market contract, both token orderbooks, Chainlink reference, Polymarket trades, and official settlement.
Out of scope
Immutable artifact publication or filesystem persistence, CLI wiring, evaluator/training changes, promotion policy, deployment, risk, and live execution.
Dependency or merge order
Depends on merged PRs #89, #93, and #94. This PR is based directly on current
main; it is not stacked on an open PR. The later artifact-publication PR must rebase onto this PR after merge.Focused validation
cargo +1.91 fmt -p hft-collector -- --checkcargo +1.91 test --locked -p hft-collector --lib --features collector-binance polymarket_research_normalize::tests— 5 passedcargo +1.91 test --locked -p hft-collector --lib --features collector-binance— 134 passed, 1 ignoredcargo +1.91 clippy --locked -p hft-collector --lib --features collector-binance --no-deps -- -D warningsRollout/rollback impact
Research-only normalization code; no runtime, deployment, promotion, or live-trading activation. Reverting this PR removes the normalizer while leaving raw validation and contract selection intact.
Scope note
2 changed files and 723 non-generated added lines, below the repository 750-line split-assessment threshold. The module and its five coupled surface builders form one normalization contract and one rollback unit.