Skip to content

Fix transient Polymarket OSS readback misses - #376

Merged
proerror77 merged 1 commit into
mainfrom
codex/polymarket-oss-readback-retry-326
Jul 26, 2026
Merged

proerror77 merged 1 commit into
mainfrom
codex/polymarket-oss-readback-retry-326

Conversation

@proerror77

@proerror77 proerror77 commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Change contract

Add a bounded retry only at the shared Polymarket Rust uploader remote-readback verification seam so a newly uploaded triplet can survive a transient first NoSuchKey without weakening fail-closed integrity checks.

This is a small, already-specified defect inside #326, so a separate PRD is unnecessary; it adds no product behavior or rollout unit.

Out of scope

Dependency or merge order

PR #372 is already merged. Merge this PR before building the next exact-main candidate and rerunning the formal Gate.

Focused validation

  • Red proof on the parent behavior: newly_uploaded_triplet_retries_transient_readback_miss failed immediately with NoSuchKey
  • cargo test -p hft-collector polymarket_upload::tests:: --lib — 43 passed
  • cargo test -p hft-collector --lib — 219 passed, 2 ignored
  • cargo test -p hft-collector --bin polymarket-raw-ops — 4 passed
  • cargo clippy -p hft-collector --lib -- -D warnings — passed
  • rustfmt --edition 2021 --check tools/collector/src/polymarket_upload.rs — passed
  • Dual read-only code review: no actionable findings

Rollout/rollback impact

Rollout: after merge, build a new exact-main Polymarket Rust candidate and rerun the formal 3600s+601s Gate before any production cutover. The Gate must still verify the exact data/manifest/_SUCCESS triplet and all existing health/parity requirements.

Rollback: revert this PR to restore single-attempt remote readback verification. No production runtime changes occur in this PR.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

OSS artifact verification now retries remote readback failures up to three times with a one-second delay. Tests cover recovery from transient missing objects, bounded failure attempts, error context, and local source-file retention.

Changes

OSS readback retry behavior

Layer / File(s) Summary
Retry verification flow
rust_hft/tools/collector/src/polymarket_upload.rs
Adds bounded retry constants and updates remote artifact verification to retry failed readbacks, preserve the last error, and report bounded exhaustion. Tests cover eventual success, attempt limits, failure messaging, and local-file cleanup or retention.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title is concise and accurately reflects the main change: adding retries for transient Polymarket OSS readback misses.
Description check ✅ Passed The description covers the contract, out-of-scope work, merge order, validation, and rollout/rollback; it only omits the Scope exception line.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/polymarket-oss-readback-retry-326

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.

Actionable comments posted: 1

🤖 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/tools/collector/src/polymarket_upload.rs`:
- Around line 1742-1752: Update the retry loop around
download_remote_artifacts_with so the expected initial NoSuchKey probe is not
retried, while retaining retries for post-upload verification failures.
Distinguish the initial readback from post-upload verification in
upload_artifacts_with or restrict this loop’s retry behavior to the verification
path, preserving immediate handling of a missing remote triplet before a new
upload.
🪄 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 Plus

Run ID: 4a1633f1-98cd-4747-866e-d9920be50110

📥 Commits

Reviewing files that changed from the base of the PR and between 67ccb95 and 93de86d.

📒 Files selected for processing (1)
  • rust_hft/tools/collector/src/polymarket_upload.rs

Comment thread rust_hft/tools/collector/src/polymarket_upload.rs
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