fix(collector): retry post-upload OSS verify per artifact on transient 404

[proerror77]

Change contract

The uploader's post-upload OSS verify retries each artifact download on explicit transient NoSuchKey/HTTP 404 failures, with one bounded 30×20s policy (~600s) for post-upload verification and a single-shot pre-upload existence probe. Every attempt and backoff observes the shared wall-clock budget; configuration, spawn, permission, and other non-404 failures fail immediately.

Out of scope

• Upload identity, manifest fields, and fix(collector): split transport reconnects from manifest request_failures #593 failure-kind semantics.
• Production redeployment of the fixed binary and the market-tape backlog drain (fix(collector): PM tape upload rejects quote_collection_failure kind, segments stuck since 2026-07-30 #533 runtime work).

Dependency or merge order

Builds on #593 (24fa4a65). Review follow-up is committed as eba34942.

Focused validation

• pre_upload_probe_is_single_shot_before_upload: a transient probe miss invokes one probe, then all three uploads start.
• remote_readback_budget_is_checked_before_attempt: zero remaining budget invokes no child process.
• remote_readback_non_404_exit_fails_without_retrying: captured permission/configuration output fails after one attempt.
• only_explicit_not_found_errors_are_retryable: unrelated text containing 404 is not retryable.
• remote_readback_retries_transient_404_until_object_is_visible: transient misses recover.
• remote_readback_retries_are_bounded: persistent misses stop at 30 attempts and retain local artifacts.
• cargo test -p hft-collector: 275 passed, 2 ignored, 0 failed.
• cargo clippy -p hft-collector --all-targets -- -D warnings: clean.
• rustfmt --edition 2021 --check tools/collector/src/polymarket_upload.rs: clean.

Rollout/rollback impact

Uploader retry behavior only; no interface change. Rollback = revert eba34942.

Issue relationship

Refs #571

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

OSS verification retry flow

Layer / File(s)	Summary
Retry policy and error classification rust_hft/tools/collector/src/polymarket_upload.rs	The uploader adds bounded per-artifact retry settings. Non-zero child-process exits are retryable. Spawn and configuration errors fail immediately.
Per-artifact download and verification flow rust_hft/tools/collector/src/polymarket_upload.rs	Downloads retry within per-file, OSS, and shared wall-clock limits. Remote verification validates artifacts after one download operation. Tests cover exhaustion, recovery, and immediate configuration failure.

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

Sequence Diagram(s)

sequenceDiagram
  participant RemoteVerification
  participant ArtifactDownloader
  participant OSSChildProcess
  RemoteVerification->>ArtifactDownloader: Download artifacts within shared wall-clock budget
  ArtifactDownloader->>OSSChildProcess: Execute artifact download
  OSSChildProcess-->>ArtifactDownloader: Return success or non-zero exit
  ArtifactDownloader->>ArtifactDownloader: Retry retryable exits up to 30 attempts
  ArtifactDownloader-->>RemoteVerification: Return artifacts or verification error

Loading

Possibly related issues

• fix(collector): polymarket-raw-ops upload oss-verify races transient 404 after PUT #571 — Both changes address transient OSS visibility failures during Polymarket uploader verification.

Possibly related PRs

• proerror77/monday#376 — Modifies the same remote artifact verification retry logic.
• proerror77/monday#565 — Revises the same OSS readback retry path in polymarket_upload.rs.
• proerror77/monday#575 — Revises the same uploader verification and readback retry logic.

🚥 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 clearly identifies the change: retrying post-upload OSS verification for transient 404 failures.
Description check	✅ Passed	The description covers the change, issue relationship, scope, dependencies, validation, and rollback; it omits only the non-applicable Scope exception section.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/uploader-oss-verify-retry

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 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 1965-1971: Update the OSS verification retry loop around the
`file_budget` and `runner` flow to check the remaining wall-clock budget before
each attempt and return the unavailable-artifact error immediately when it is
exhausted. Before the retry sleep in the `attempt` handling block, recompute the
remaining budget and cap the sleep duration at that value so verification cannot
continue beyond `OSS_READBACK_MAX_WALL_CLOCK`.
- Around line 1949-1964: The download helper’s retry count must be configurable
so the normal pre-upload existence probe is not retried. Add an explicit
attempt-count parameter or policy to the helper containing the loop over
OSS_VERIFY_DOWNLOAD_ATTEMPTS, pass one attempt from
remote_artifacts_exist_and_match_with, and retain the 30-attempt policy from
verify_remote_artifacts_with. Add a test covering a retryable non-zero probe
exit and assert that upload begins after the single probe.

🪄 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: 84dc7c18-5385-4bfd-8613-46e6ec0795d6

📥 Commits

Reviewing files that changed from the base of the PR and between 24fa4a6 and 615a9f2.

📒 Files selected for processing (1)

• rust_hft/tools/collector/src/polymarket_upload.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 615a9f2f48

ℹ️ 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".