fix(pm-collector): bound incomplete-market trade-ID retention

[proerror77]

Change contract

Stop the reference collector's 4M-cap crash loop: bound incomplete-market trade-ID retention by evicting the oldest incomplete IDs before the cap is reached. Refs #589.

compact_trade_dedupe only compacts completed-market IDs; incomplete markets retain every ID forever. Production state on 2026-08-02: 3,998,592 retained IDs — all on incomplete markets, oldest 7 days — crashing every ~24 min and killing shadow-gate baseline checks. At ~550k IDs/hour, no age-based horizon fits the cap.

• evict_oldest_incomplete_trade_ids keeps retained IDs ≤ 3/4 of the cap, oldest-first; complete-market IDs are never evicted.
• Applied at startup recovery and every cycle before validate_state_bounds (which stays fail-closed for overflow eviction cannot cover).
• New trade_ids_evicted health field for observability (additive; the gate's health policy is not closed-world).

Issue relationship

Refs #589

Out of scope

• Why 9.4k markets stay trade-incomplete (fix(deploy): drop legacy-Python health admission from polymarket shadow gate #553).
• Gate preflight budget for tick segments (fix(pm-gate): real-market preflight budget too small for tick-level segments #586).

Dependencies and merge order

None. Unblocks the pending raw-ops cutover (candidate rebuild → gate → cutover).

Focused validation

• cargo test -p hft-collector polymarket_raw:: — 78 passed, 0 failed.
• Counterexample tests: oldest-first eviction spares complete-market IDs and clears emptied conditions; no-op under budget; complete-market overflow still fails closed.

Rollout and rollback

• Rebuild raw-ops candidate from the merged SHA → shadow gate → cutover. The new binary's startup pass prunes the oversized state below the cap on first boot.
• Rollback = previous release; the pruned state file remains valid (fewer dedupe IDs only).

Scope exception

None

Summary by CodeRabbit

• New Features

  • Added support for trade-collection completion records without affecting parity comparisons.
  • Added automatic cleanup of older trade IDs to manage retention limits.
  • Preserved records for completed markets during cleanup.
  • Added eviction counts to health reporting.

• Bug Fixes

  • Improved handling of trade-ID storage overflow while retaining safeguards for unresolved limits.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 39 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 Plus

Run ID: 94dea8df-aefb-4e4b-8702-43dc0131b4f7

📥 Commits

Reviewing files that changed from the base of the PR and between cc2a07a and 1afaba6.

📒 Files selected for processing (1)

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

📝 Walkthrough

Walkthrough

The PR adds parity handling for trade-collection completion records and introduces budget-based eviction for incomplete-market trade IDs. Startup and collection paths apply eviction, persist startup changes, report evictions, and validate overflow behavior.

Changes

Parity completion records

Layer / File(s)	Summary
Completion record parity handling rust_hft/tools/collector/src/polymarket_parity.rs	The parser accepts polymarket_trade_collection_complete records, retains them in tapes, and excludes them from parity comparison windows. Tests cover differing condition IDs.

Trade-ID retention eviction

Layer / File(s)	Summary
Incomplete-market trade-ID eviction rust_hft/tools/collector/src/polymarket_raw.rs	The collector evicts oldest incomplete-market IDs after the 75% budget threshold. Startup and collection paths apply eviction. Startup persists changes, health output reports eviction counts, and tests cover cleanup, no-op behavior, and complete-market overflow failures.

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

Possibly related issues

• fix(pm-parity): admit trade-completion records in shadow parity verification #587: Adds parity handling for polymarket_trade_collection_complete records.
• fix(pm-collector): bound incomplete-market trade-ID retention to stop cap crash loop #589: Adds incomplete-market trade-ID eviction and overflow-bound preservation.

Possibly related PRs

• proerror77/monday#588: Directly relates to the same parity completion-record handling.
• proerror77/monday#576: Directly relates to retained trade-ID capacity and validation behavior.
• proerror77/monday#34: Introduced related Polymarket parity and raw collector functionality.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: bounding incomplete-market trade-ID retention in the Polymarket collector.
Description check	✅ Passed	The description includes all required template sections and provides clear behavior, rationale, validation, rollout, rollback, scope, and issue details.
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 💡 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/pm-trade-id-budget

---

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: 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_raw.rs`:
- Around line 2049-2058: Update the eviction candidate selection around
state.trade_seen to use a bounded max-heap containing timestamps and references
to condition/trade IDs, retaining only the oldest excess entries without cloning
every candidate. After selection, clone the condition_id and trade_id only for
the chosen victims before applying eviction, preserving the existing incomplete
filtering and age ordering.

🪄 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: 4db17ed0-7dd0-42e8-b2b3-8832fef43861

📥 Commits

Reviewing files that changed from the base of the PR and between 01407ad and cc2a07a.

📒 Files selected for processing (2)

• rust_hft/tools/collector/src/polymarket_parity.rs
• rust_hft/tools/collector/src/polymarket_raw.rs

[chatgpt-codex-connector]

💡 Codex Review

monday/rust_hft/tools/collector/src/polymarket_raw.rs

Lines 2061 to 2063 in cc2a07a

	for (_, condition_id, trade_id) in by_age.into_iter().take(excess) {
	if let Some(seen) = state.trade_seen.get_mut(&condition_id) {
	if seen.remove(&trade_id).is_some() {

Keep IDs that the next full poll can replay

When an evicted condition still returns those trades—particularly for markets with fewer than the 20,000 rows fetched in full—this removal forgets IDs that the next poll immediately retrieves again. trade_updates then treats them as new, writes duplicate record IDs, and resets the finalization counter because new_trade_count > 0; under cap pressure, affected markets can therefore remain permanently incomplete while producing tape segments that uploader/recovery rejects as duplicates. The focused tests exercise deletion only in isolation and need a delete-then-refetch counterexample.

AGENTS.md reference: AGENTS.md:L115-L116

---

monday/rust_hft/tools/collector/src/polymarket_parity.rs

Lines 337 to 340 in cc2a07a

	// Trade completion proofs are per-lane polling bookkeeping emitted on
	// independent schedules, not shared market evidence; admit them in the
	// tape but exclude them from the comparison window.
	Some("polymarket_trade_collection_complete") => Ok(false),

Validate completion rows before excluding them

For any completion row, this branch returns before schema validation; parse_tape_row checks only that the update is an object with an allowed kind. Parity consequently passes proofs missing required fields such as record_id_version, trade_count, the record-ID digest, and stable_polls_required—as the newly added test demonstrates—even though the uploader and validate_reference_tape_for_recovery reject the same tape. Validate the per-lane proof contract before excluding it from cross-lane comparison.

AGENTS.md reference: AGENTS.md:L115-L116

---

monday/rust_hft/tools/collector/src/polymarket_raw.rs

Lines 2049 to 2055 in cc2a07a

	let mut by_age: Vec<(i64, String, String)> = Vec::new();
	for (condition_id, seen) in &state.trade_seen {
	if !incomplete.contains(condition_id) {
	continue;
	}
	for (trade_id, timestamp) in seen {
	by_age.push((*timestamp, condition_id.clone(), trade_id.clone()));

Avoid cloning every retained ID during eviction

At the production trigger of roughly four million retained IDs, this builds and sorts a vector containing a cloned condition ID and cloned 64-byte trade ID for every incomplete entry before removing only the excess. The vector alone requires hundreds of megabytes for tuples and cloned buffers, while normal cycles already hold both self.state and a full next_state clone; the inspected production and shadow units cap memory at 2048M, and the shadow gate rejects even a 1536M MemoryHigh event. The pruning pass can therefore hit the exact memory gate or OOM it is intended to prevent; select only the required oldest entries with bounded auxiliary storage instead.

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