feat(polymarket): gate quote tape completeness - #239
Conversation
|
Warning Review limit reached
Next review available in: 20 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 Plus Run ID: 📒 Files selected for processing (3)
✨ 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: 42c195b70c
ℹ️ 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 mut manifest = json!({ | ||
| "schema": "monday.polymarket.raw.v1", | ||
| "canonical": reference_context_complete, | ||
| "canonical": reference_context_complete && quote_coverage_complete, |
There was a problem hiding this comment.
When an upload contains both complete and incomplete chunks, this correctly marks the latter noncanonical, but the inspected shadow gate at deployment/aliyun/polymarket-raw-ops-shadow-gate.sh:902-906 and :950-957 only requires each canonical count to be greater than zero. Consequently, one complete chunk allows production_eligible to remain true despite any number of chunks failing the new quote checks; compare the canonical count with the total uploaded count so a mixed run is rejected.
AGENTS.md reference: AGENTS.md:L93-L96
Useful? React with 👍 / 👎.
| if matches!(kind, "quote" | "quote_collection_failure") { | ||
| let token = required_text(update, "token_id", line_number)?; | ||
| attempted_quote_token_ids.insert(token.to_owned()); | ||
| request_attempts += 1; |
There was a problem hiding this comment.
For each WebSocket failure, send_quote_collection_failure_and_empty emits a QuoteCollectionFailure followed by a synthetic empty Quote for the same actual attempt (ploy-market-data/src/feeds.rs:1149-1177). This block increments request_attempts for both records, while the later request_successes: quote_count also reports the synthetic empty quote as a success, so one failed request is recorded as two attempts—one successful and one failed—corrupting the manifest's collection-reliability metrics.
Useful? React with 👍 / 👎.
| let missing_quote_tokens = known_event_tokens | ||
| .difference("ed_token_ids) | ||
| .cloned() |
There was a problem hiding this comment.
When an event_discovered record omits an outcome token, supplies an empty token, or repeats the same token for both outcomes, the earlier population of known_event_tokens silently skips or deduplicates it. This difference therefore has no way to report that outcome in missing_quote_tokens, allowing quotes for only the remaining token to produce quote_coverage_complete=true and a canonical manifest for an incomplete or malformed market; require two nonempty, distinct outcome tokens before using this set as the coverage denominator.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
| let quote_quality_complete = incomplete_quotes == 0 | ||
| && crossed_quotes == 0 | ||
| && out_of_range_prices == 0 | ||
| && negative_sizes == 0; |
There was a problem hiding this comment.
A malformed quote can provide a valid noncrossed top level such as bid=0.49, ask=0.51 while its preserved depth contains a tradeable bid at 0.90 and ask at 0.10. The new scan validates each depth level only in isolation and derives crossed_quotes solely from the supplied top fields, so this tape receives quote_quality_complete=true and can become canonical even though replaying its full-depth book yields a crossed market; validate depth ordering and ensure the best tradeable depth levels agree with the top quote.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
| if last_quote_source_at | ||
| .insert(token.to_owned(), source_at) | ||
| .is_some_and(|previous| source_at < previous) | ||
| { | ||
| bail!("line {line_number}: quote source time moved backwards"); |
There was a problem hiding this comment.
When a WebSocket reconnect returns a fresh snapshot whose exchange timestamp is older than the last snapshot from the failed connection, the producer accepts it because spawn_clob_ws_quote_feed_until clears its per-token timestamp map at the reconnect boundary. The uploader retains last_quote_source_at across the intervening quote_collection_failure, however, so it rejects that producer-valid recovery tape with quote source time moved backwards and leaves the closed segment pending; clear the affected token's ordering state when processing the explicit failure boundary.
Useful? React with 👍 / 👎.
89697e5 to
b86a386
Compare
b86a386 to
8e8fa65
Compare
Change contract: Polymarket raw-tape validation independently recomputes quote classification, coverage, latency, field quality, and collection failures, and only counts complete manifests as canonical.
Out of scope: Production deployment/cutover, snapshot construction, evaluator/MCTS, and research logic are unchanged.
Dependency or merge order: Producer contract merged first in PR #238. This validation PR is rebased directly onto main.
Focused validation: hft-collector Polymarket uploader tests (41 passed); bash syntax; shellcheck; Polymarket raw-ops control-plane test; new-ploy-runner full-feature cargo check; final read-only spec and standards reviews found no blocker.
Rollout/rollback impact: No deployment is performed. Noncanonical raw remains preserved with explicit quality evidence, but canonical upload counts and the shadow gate fail closed for missing context, coverage, timestamps, unexplained fields, or collection failures. Revert this PR independently; producer recording remains intact.
Closes #220.