Skip to content

fix(polymarket): bound cold-start retained values - #86

Merged
proerror77 merged 1 commit into
mainfrom
codex/polymarket-discovery-target-retention
Jul 17, 2026
Merged

fix(polymarket): bound cold-start retained values#86
proerror77 merged 1 commit into
mainfrom
codex/polymarket-discovery-target-retention

Conversation

@proerror77

@proerror77 proerror77 commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Change contract

Reduce the Polymarket reference collector cold-start retained Value peak by retaining only configured target markets and moving owned market/trade payloads through the cycle, while preserving output schema, ordering, pagination, limits, and fail-closed completeness.

Out of scope

  • No MemoryHigh, MemoryMax, CPU, systemd, deployment, or gate-policy changes.
  • No change to max_markets, trade polling budgets, target eligibility, settlement rules, or event-local completeness.
  • No streaming-write redesign; updates remain durably published with the existing atomic cycle contract.

Dependency or merge order

None. This branch is rebased onto main after #85 (672M/768M calibration).

Focused validation

  • Counterexample tests prove non-target Gamma rows are validated and count toward independent lane caps without being retained.
  • Target rows remain retained across open and closed lanes; malformed rows and unexhausted cursors still fail closed.
  • Raw Data API pages, trade vectors, target values, and raw trade payloads are consumed rather than deep-cloned; the full raw trade payload assertion remains identical.
  • cargo fmt -p hft-collector -- --check
  • CARGO_BUILD_JOBS=1 cargo test -p hft-collector --features collector-binance --locked (149 passed across lib and binaries, 2 network/subprocess targets ignored as designed)
  • CARGO_BUILD_JOBS=1 cargo clippy -p hft-collector --all-targets --features collector-binance --no-deps --locked -- -D warnings

Rollout/rollback impact

The collector emits the same records and health target count with a lower cold-start memory peak. Rollout requires a new Linux collector artifact and the existing shadow gate. Rollback is a normal revert to the previous collector artifact; no data or configuration migration is required.

Summary by CodeRabbit

  • Bug Fixes
    • Improved market discovery to honor configured symbols and market limits more accurately.
    • Prevented non-target markets from being included in collected data.
    • Improved trade update handling so complete trade details are retained.
    • Added stronger validation for trade data responses.
    • Improved collection health reporting when markets are processed in batches.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Gamma market discovery now filters configured symbols and enforces per-lane limits. Trade collection validates owned response data, preserves full trade payloads, and target processing drains owned entries while retaining accurate health metrics.

Changes

Polymarket collector

Layer / File(s) Summary
Gamma discovery filtering and caps
rust_hft/tools/collector/src/polymarket_raw.rs
Gamma discovery tracks seen and target markets separately, filters by configured symbols, enforces lane-specific caps, and updates related tests.
Owned trade parsing and update emission
rust_hft/tools/collector/src/polymarket_raw.rs
Trade pages must be arrays, object rows and trade updates use owned values, deduplication borrows trades for IDs, and emitted updates retain the complete trade payload.
Target ownership and health accounting
rust_hft/tools/collector/src/polymarket_raw.rs
Collection chunks owned target IDs, removes targets during processing, updates market-operation arguments, and reports the captured target count.

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

Possibly related PRs

  • proerror77/monday#37: Overlaps in the Polymarket trade collection, polling, and collect_once() processing paths.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: bounding retained values in the Polymarket collector.
Description check ✅ Passed The description covers the required sections with concrete validation, rollout, and scope details; only the optional scope-exception note is omitted.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/polymarket-discovery-target-retention

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/tools/collector/src/polymarket_raw.rs (1)

1153-1160: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Prefer in-place filtering to avoid vector reallocation.

Since page is already owned, you can use .retain() to filter out non-objects in place. This avoids allocating a new vector and fits perfectly with the PR's goal of efficiently consuming the raw payloads.

♻️ Proposed fix using `retain`
-fn object_rows(page: Vec<Value>) -> (Vec<Value>, u64) {
-    let page_len = page.len();
-    let objects = page
-        .into_iter()
-        .filter(|value| value.is_object())
-        .collect::<Vec<_>>();
-    let rejected = u64::try_from(page_len - objects.len()).unwrap_or(u64::MAX);
-    (objects, rejected)
-}
+fn object_rows(mut page: Vec<Value>) -> (Vec<Value>, u64) {
+    let page_len = page.len();
+    page.retain(Value::is_object);
+    let rejected = u64::try_from(page_len - page.len()).unwrap_or(u64::MAX);
+    (page, rejected)
+}
🤖 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/tools/collector/src/polymarket_raw.rs` around lines 1153 - 1160,
Update object_rows to filter the owned page in place with retain, removing
non-object values without collecting into a new vector. Preserve the rejected
count based on the original and retained lengths, then return the filtered page
and count.
🤖 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/tools/collector/src/polymarket_raw.rs`:
- Around line 1153-1160: Update object_rows to filter the owned page in place
with retain, removing non-object values without collecting into a new vector.
Preserve the rejected count based on the original and retained lengths, then
return the filtered page and count.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 32af1b76-9078-489f-a052-d0832c630c13

📥 Commits

Reviewing files that changed from the base of the PR and between e936881 and 6230334.

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

@proerror77
proerror77 merged commit f7f11a9 into main Jul 17, 2026
18 checks passed
@proerror77
proerror77 deleted the codex/polymarket-discovery-target-retention branch July 17, 2026 09:11
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