Skip to content

fix(collector): bound explicit historical discovery - #172

Merged
proerror77 merged 1 commit into
mainfrom
codex/polymarket-bounded-discovery
Jul 20, 2026
Merged

proerror77 merged 1 commit into
mainfrom
codex/polymarket-bounded-discovery

Conversation

@proerror77

@proerror77 proerror77 commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Closes #171

Change contract

When an isolated reference collector run supplies one or more --market-id values, fetch and identity-check only those Gamma market-detail records instead of scanning broad keyset pages. Without explicit IDs, retain the existing open plus closed 24-hour discovery behavior.

Out of scope

New date-range flags, completion semantics, polling limits, production ECS/reference-recovery cutover, evidence publication, snapshots, evaluator/MCTS, and live execution.

Dependency or merge order

None. This corrects the opt-in interface introduced by merged #166 and is based on current main (8e5bb9b2).

Focused validation

  • cargo test -p hft-collector requested_market_ids_bypass_broad_gamma_discovery --lib --no-default-features --locked
  • cargo test -p hft-collector requested_non_target_market_fails_before_completion --lib --no-default-features --locked
  • cargo test -p hft-collector settlement_discovery_capacity_covers_every_configured_five_and_fifteen_minute_market --lib --no-default-features --locked
  • cargo test -p hft-collector --bin polymarket-raw-ops --no-default-features --locked
  • cargo clippy -p hft-collector --lib --no-default-features --locked -- -D warnings
  • git diff --check

The repository-wide cargo fmt --check reports pre-existing formatting diffs outside this PR; no unrelated formatting was changed.

Rollout/rollback impact

Publish a content-addressed binance-lob-archiver image and run it only in a named isolated Kubernetes Job for the known BTC 5m cohort. No production collector changes. Roll back by deleting that Job and retaining the current image.

PRD note

A new PRD is unnecessary: this is a narrow correction to the already-merged explicit-market-id collector contract. #171 provides the scoped contract and acceptance evidence.

Summary by CodeRabbit

  • New Features

    • Added support for retrieving market details directly when specific market IDs are configured.
    • Added validation to prevent requesting more markets than the configured maximum.
  • Bug Fixes

    • Prevented unrelated markets from being included when specific market IDs are requested.
    • Improved handling of invalid market requests before trade output is generated.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 08f0ebe0-2ae6-48a5-8bc4-dd59055762f7

📥 Commits

Reviewing files that changed from the base of the PR and between 8e5bb9b and 26f166c.

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

📝 Walkthrough

Walkthrough

Explicit market IDs are now capacity-validated and discovered through direct Gamma market-detail requests. Tests cover bypassing keyset pagination and rejecting unsupported markets before completion output.

Changes

Explicit Polymarket market discovery

Layer / File(s) Summary
Validate and fetch explicit markets
rust_hft/tools/collector/src/polymarket_raw.rs
ReferenceConfig::validate rejects explicit ID sets larger than max_markets; discover_markets fetches configured IDs directly from Gamma and skips keyset pagination.
Test bounded discovery and early failure
rust_hft/tools/collector/src/polymarket_raw.rs
Tests cover the capacity limit, direct market-detail requests, and failure before trade-completion output for non-target markets.

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

Sequence Diagram(s)

sequenceDiagram
  participant collect_once
  participant ReferenceCollector
  participant GammaAPI
  participant ActiveTape
  collect_once->>ReferenceCollector: discover configured market IDs
  ReferenceCollector->>GammaAPI: GET /markets/{id}
  GammaAPI-->>ReferenceCollector: market detail
  ReferenceCollector->>ReferenceCollector: validate supported target
  ReferenceCollector->>ActiveTape: write completion output only after successful collection
Loading

Possibly related PRs

  • proerror77/monday#86: Both changes modify Polymarket Gamma discovery bounds and configured market-ID validation.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the collector change and matches the explicit-market discovery fix.
Description check ✅ Passed All required sections are present and the description covers scope, validation, rollout, and out-of-scope items.
Linked Issues check ✅ Passed The diff matches #171 by bypassing keyset scans for explicit market IDs, adding bounded validation, and covering default and failure cases with tests.
Out of Scope Changes check ✅ Passed No obvious out-of-scope code changes are present; the diff stays within bounded collector discovery and related tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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-bounded-discovery

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.

@proerror77
proerror77 merged commit 65cf635 into main Jul 20, 2026
18 of 19 checks passed
@proerror77
proerror77 deleted the codex/polymarket-bounded-discovery 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.

Bound closed-market discovery for isolated Polymarket research backfills

1 participant