Skip to content

fix: accept causal batched Polymarket completions - #211

Merged
proerror77 merged 2 commits into
mainfrom
codex/polymarket-completion-causal-clock
Jul 22, 2026
Merged

fix: accept causal batched Polymarket completions#211
proerror77 merged 2 commits into
mainfrom
codex/polymarket-completion-causal-clock

Conversation

@proerror77

@proerror77 proerror77 commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Change contract

Accept causal Polymarket trade-completion retrievals that occur before a batched segment is recorded, while enforcing the fail-closed window segment UTC hour start <= retrieved_at <= end_recorded_at.

Out of scope

Collector/importer changes, evidence bytes, permission or digest checks, snapshot/cohort construction, evaluator/MCTS, Paper/Live, 15m/1h.

Dependency / merge order

Base: main. No stacked code dependency. This focused fix blocks cloud completion of #173 and therefore #169. Relates to #210; keep #210 open until the fresh ACK verifier proof passes.

Focused validation

  • Red proof: seals_completion_retrieved_before_its_batched_segment_is_recorded failed under the prior lower bound.
  • Boundary tests: same-hour causal batch accepted; before-hour backdating rejected; after-recorded-end future timestamp rejected.
  • cargo test -p ploy-market-data — 48 passed.
  • cargo fmt -p ploy-market-data -- --check — passed.
  • cargo clippy -p ploy-market-data --all-targets --no-deps -- -D warnings — passed.
  • Two independent read-only code reviews — no remaining findings.
  • Full dependency clippy remains blocked by three pre-existing ploy-market-contracts derivable-impl warnings; this PR does not own them.

Rollout / rollback impact

Rollout publishes a new immutable research-runner image, then reruns the existing read-only ACK typed-verifier Job using exact evidence digests. Rollback is a one-commit revert; no data migration or collector change.

Why no PRD

This is a small, already-specified verifier defect with one temporal predicate and targeted counterexamples, so Matt TDD is the narrowest workflow.

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation of trade-completion timestamps in reference datasets.
    • Completions are now correctly accepted or rejected based on their position within the segment’s enclosing hour and end time.
    • Added coverage for edge cases involving segment recording times and hourly boundaries.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@proerror77, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 825bc418-911a-40e6-8da3-7da8406e009c

📥 Commits

Reviewing files that changed from the base of the PR and between 821576c and 42c7fcf.

📒 Files selected for processing (2)
  • rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.json
  • rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.json
📝 Walkthrough

Walkthrough

Reference segment validation now derives the segment hour start and uses it for trade-completion retrieval checks. Tests cover retrieval before recording, after recording, and before the enclosing hour.

Changes

Reference timestamp validation

Layer / File(s) Summary
Derive and pass segment hour start
rust_hft/.../polymarket_evidence/artifact.rs
validate_segment parses the RFC3339 hour start from the segment date and hour, then passes it to validate_trade_completions.
Enforce retrieval bounds and edge cases
rust_hft/.../polymarket_evidence/artifact.rs
Completion validation compares retrieved_at with the enclosing hour start while retaining end-bound and completeness checks; tests cover retrieval timing boundaries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • Issue 210 — Both changes update Polymarket trade-completion timestamp validation to allow retrieval before batched segment recording while preserving the segment end bound.

Possibly related PRs

  • proerror77/monday#152 — Introduced related validate_trade_completions and trade-completion proof binding in the same module.
  • proerror77/monday#154 — Validated ordered Polymarket reference segment sets used by this contract.
  • proerror77/monday#175 — Adjusted Polymarket reference segment hour and time validation for same-hour data.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: allowing causal batched Polymarket completions.
Description check ✅ Passed All major template sections are present, but the required Scope exception section is replaced by an off-template 'Why no PRD' note.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/polymarket-completion-causal-clock

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rs (1)

1377-1436: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add an explicit lower-bound equality test.

The new tests cover values below and above the bounds, but not retrieved_at == hour_start. Add that case to protect the required inclusive lower bound.

🤖 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/artifact.rs`
around lines 1377 - 1436, Add a test alongside
rejects_completion_retrieved_before_its_segment_hour that sets
trade_completions["market-1"].retrieved_at exactly to the segment hour start,
invokes seal_polymarket_evidence_triplet, and asserts successful sealing.
Preserve the existing below- and above-bound rejection tests.
🤖 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/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rs`:
- Around line 1377-1436: Add a test alongside
rejects_completion_retrieved_before_its_segment_hour that sets
trade_completions["market-1"].retrieved_at exactly to the segment hour start,
invokes seal_polymarket_evidence_triplet, and asserts successful sealing.
Preserve the existing below- and above-bound rejection tests.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1d637af-ee2f-4327-af84-a118fcdfce62

📥 Commits

Reviewing files that changed from the base of the PR and between 8ba6d7a and 821576c.

📒 Files selected for processing (1)
  • rust_hft/prediction-markets/crates/ploy-market-data/src/polymarket_evidence/artifact.rs

@proerror77
proerror77 merged commit 344a9bf into main Jul 22, 2026
26 checks passed
@proerror77
proerror77 deleted the codex/polymarket-completion-causal-clock branch July 24, 2026 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant