fix: polymarket deprecated endpoint update#233
Conversation
nikbpetrov
force-pushed
the
polymarket-api-deprecation
branch
from
19a9225 to
f5a3f87
Compare
houtanb
force-pushed
the
polymarket-api-deprecation
branch
from
f5a3f87 to
5298f1b
Compare
📝 WalkthroughWalkthroughMigrates Polymarket market retrieval to the ChangesPolymarket API pagination and request handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sources/polymarket.py (1)
259-297: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftNo retry/backoff for the pagination loop; transient errors silently truncate results.
_get_marketand_fetch_price_historyboth use@backoff.on_exceptionforRequestException, but this loop just logs andbreaks on the first failure, returning whatever pages were fetched so far without raising. Sincefetch()(line 90-92) treats the returned list as the complete set of newly active markets, a single mid-pagination network blip can silently drop qualifying markets from the dataset with no error surfaced upstream.Consider wrapping each page fetch with the same
@backoff.on_exceptionpattern used elsewhere in this file, or at minimum re-raising after logging so callers are aware of a partial fetch.🤖 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 `@src/sources/polymarket.py` around lines 259 - 297, The pagination loop in the Polymarket market fetch logic currently logs a RequestException and breaks, which can silently return a partial market list. Update the page-fetch path in the market retrieval method to use the same retry/backoff approach as `_get_market` and `_fetch_price_history`, or re-raise the exception after logging instead of breaking, so `fetch()` does not treat an incomplete result as a successful full fetch.
🧹 Nitpick comments (2)
src/tests/test_polymarket.py (1)
390-412: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for the
RequestExceptionbranch in the pagination loop.
_fetch_active_markets_from_apinow has atry/except requests.exceptions.RequestExceptionpath that breaks and returns partial results (source lines 295-297), but no test in this class exercises it. Given the partial-results/data-loss concern raised on the source file, test coverage here would help lock in whatever behavior is decided.🤖 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 `@src/tests/test_polymarket.py` around lines 390 - 412, Add a test for the RequestException path in PolymarketSource._fetch_active_markets_from_api so the pagination loop’s try/except branch is covered. Extend the existing pagination tests in test_pagination or add a sibling test that patches sources.polymarket.requests.get to succeed once and then raise requests.exceptions.RequestException, then assert the method returns the partial markets collected before the failure and stops making further requests.src/sources/polymarket.py (1)
294-305: 🩺 Stability & Availability | 🔵 TrivialConsider a safety cap on pagination iterations.
Termination relies entirely on the API omitting
next_cursoron the last page. Given this is a newly-migrated endpoint, a defensive max-iteration/max-page guard would prevent a runaway loop if the API ever returns a stale/repeating cursor.🤖 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 `@src/sources/polymarket.py` around lines 294 - 305, The pagination loop in the markets-fetching flow needs a defensive cap so it cannot run forever if the API repeats a stale cursor. Update the logic around the `after_cursor`/`next_cursor` handling in the markets retrieval method to track page or iteration count and stop once a reasonable maximum is reached. Keep the existing `next_cursor` termination path, but add a clear log and exit condition in the same loop that fetches pages and appends to `all_markets`.
🤖 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 `@src/sources/polymarket.py`:
- Around line 256-266: The paginated Gamma API fetch in the market-loading loop
uses requests.get without a timeout, which can block the loop indefinitely.
Update the request in the market retrieval logic to pass an explicit timeout,
and keep it consistent across all iterations so a slow or hung API response
cannot stall the fetch process. Use the existing logger/info flow and the
requests.get call inside the market pagination loop as the place to apply the
fix.
---
Outside diff comments:
In `@src/sources/polymarket.py`:
- Around line 259-297: The pagination loop in the Polymarket market fetch logic
currently logs a RequestException and breaks, which can silently return a
partial market list. Update the page-fetch path in the market retrieval method
to use the same retry/backoff approach as `_get_market` and
`_fetch_price_history`, or re-raise the exception after logging instead of
breaking, so `fetch()` does not treat an incomplete result as a successful full
fetch.
---
Nitpick comments:
In `@src/sources/polymarket.py`:
- Around line 294-305: The pagination loop in the markets-fetching flow needs a
defensive cap so it cannot run forever if the API repeats a stale cursor. Update
the logic around the `after_cursor`/`next_cursor` handling in the markets
retrieval method to track page or iteration count and stop once a reasonable
maximum is reached. Keep the existing `next_cursor` termination path, but add a
clear log and exit condition in the same loop that fetches pages and appends to
`all_markets`.
In `@src/tests/test_polymarket.py`:
- Around line 390-412: Add a test for the RequestException path in
PolymarketSource._fetch_active_markets_from_api so the pagination loop’s
try/except branch is covered. Extend the existing pagination tests in
test_pagination or add a sibling test that patches
sources.polymarket.requests.get to succeed once and then raise
requests.exceptions.RequestException, then assert the method returns the partial
markets collected before the failure and stops making further requests.
🪄 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: e0bd0532-d76b-41bc-b383-27fa364cb880
📒 Files selected for processing (2)
src/sources/polymarket.pysrc/tests/test_polymarket.py
nikbpetrov
force-pushed
the
polymarket-api-deprecation
branch
from
5298f1b to
1f9343f
Compare
There was a problem hiding this comment.
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 `@src/sources/polymarket.py`:
- Around line 240-241: Ensure _fetch_price_history does not swallow
RequestException: re-raise a retryable exception so its backoff wrapper retries,
or make _fetch_active_markets_from_api treat a None result as a hard failure
instead of silently dropping the market. Update
test_skips_when_price_history_none to assert the revised failure and retry
behavior.
🪄 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: c56dcc66-16c1-4324-8dad-1592d6bae98d
📒 Files selected for processing (2)
src/sources/polymarket.pysrc/tests/test_polymarket.py
| and attaches each qualifying market's price history. A transient request failure retries | ||
| the whole paginated fetch via backoff rather than returning a truncated result. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== outline ==\n'
ast-grep outline src/sources/polymarket.py --view expanded || true
printf '\n== search symbols ==\n'
rg -n "_fetch_price_history|backoff|test_skips_when_price_history_none|RequestException|price history" src tests || true
printf '\n== line ranges ==\n'
nl -ba src/sources/polymarket.py | sed -n '210,330p'Repository: forecastingresearch/forecastbench
Length of output: 11274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== polymarket slices ==\n'
sed -n '229,310p' src/sources/polymarket.py
printf '\n== fetch_price_history slice ==\n'
sed -n '348,398p' src/sources/polymarket.py
printf '\n== relevant tests slice ==\n'
sed -n '430,530p' src/tests/test_polymarket.pyRepository: forecastingresearch/forecastbench
Length of output: 10022
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '130,175p' src/sources/polymarket.py
sed -n '175,235p' src/sources/polymarket.pyRepository: forecastingresearch/forecastbench
Length of output: 4660
Do not swallow price-history failures
_fetch_price_history catches RequestException and returns None, so its backoff wrapper never retries. In _fetch_active_markets_from_api, that None just drops the market, which leaves the fetched set incomplete instead of retrying the failing request. Re-raise a retryable exception here, or handle None as a hard failure, and update test_skips_when_price_history_none accordingly.
🤖 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 `@src/sources/polymarket.py` around lines 240 - 241, Ensure
_fetch_price_history does not swallow RequestException: re-raise a retryable
exception so its backoff wrapper retries, or make _fetch_active_markets_from_api
treat a None result as a hard failure instead of silently dropping the market.
Update test_skips_when_price_history_none to assert the revised failure and
retry behavior.
| "order": "liquidityNum", | ||
| "ascending": False, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the cited lines and relevant helpers.
python3 - <<'PY'
from pathlib import Path
path = Path("src/sources/polymarket.py")
lines = path.read_text().splitlines()
for start, end in [(1, 360)]:
for i in range(start-1, min(end, len(lines))):
if 220 <= i+1 <= 340:
print(f"{i+1:4d}: {lines[i]}")
PYRepository: forecastingresearch/forecastbench
Length of output: 6231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map nearby symbols to focus on pagination and retry behavior.
ast-grep outline src/sources/polymarket.py --view expandedRepository: forecastingresearch/forecastbench
Length of output: 1368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect price-history handling and surrounding retry logic.
python3 - <<'PY'
from pathlib import Path
path = Path("src/sources/polymarket.py")
lines = path.read_text().splitlines()
for start, end in [(350, 390)]:
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: forecastingresearch/forecastbench
Length of output: 1947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Continue to the exception handling and any caller behavior.
python3 - <<'PY'
from pathlib import Path
path = Path("src/sources/polymarket.py")
lines = path.read_text().splitlines()
for start, end in [(389, 405)]:
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: forecastingresearch/forecastbench
Length of output: 1035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the rest of the price-history exception path.
python3 - <<'PY'
from pathlib import Path
path = Path("src/sources/polymarket.py")
lines = path.read_text().splitlines()
for start, end in [(389, 405)]:
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: forecastingresearch/forecastbench
Length of output: 1035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the remainder of the price-history exception path to see how failures are surfaced.
python3 - <<'PY'
from pathlib import Path
path = Path("src/sources/polymarket.py")
lines = path.read_text().splitlines()
for start, end in [(389, 410)]:
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: forecastingresearch/forecastbench
Length of output: 1257
🌐 Web query:
Polymarket Gamma API markets keyset pagination order after_cursor liquidityNum docs
💡 Result:
The Polymarket Gamma API utilizes keyset (cursor-based) pagination for efficient data retrieval, explicitly rejecting offset-based pagination [1][2][3]. Keyset Pagination Mechanics: - Use the after_cursor query parameter to request subsequent pages [1][3]. - The API response contains a next_cursor field; pass this value into the after_cursor parameter of the next request [1][3]. - Note: Offset-based pagination is not supported and will return an HTTP 422 error [1][3]. - Technical reports as of April 2026 have noted potential issues where the keyset endpoint may fail to advance past the first page when the cursor parameter is provided [4]. Developers are encouraged to use the provided TypeScript or Python SDKs, which abstract these pagination patterns [5][6]. LiquidityNum: - liquidityNum represents a market's liquidity expressed as a numerical value [7][8][9]. - It is available as a filterable field in various endpoints (e.g., liquidity_num_min, liquidity_num_max) to refine market search results [10][9]. General API Tips: - The Gamma API base URL is https://gamma-api.polymarket.com [3]. - SDKs for TypeScript and Python are available to simplify handling of pagination, typing, and request flows [5][6].
Citations:
- 1: https://docs.polymarket.com/api-reference/markets/list-markets-keyset-pagination
- 2: https://polymarket-docs.copilot.markets/api-reference/markets/list-markets-keyset-pagination
- 3: https://www.polymarketarbitragebot.net/guides/polymarket-gamma-api-tutorial
- 4: gamma-api keyset endpoints: cursor parameter ignored, returns same first page Polymarket/agents#227
- 5: https://docs.polymarket.com/dev-tooling/typescript
- 6: https://docs.polymarket.com/dev-tooling/python
- 7: https://docs.polymarket.us/api-reference/market/overview
- 8: https://docs.polymarket.com/api-reference/markets/get-market-by-slug
- 9: https://docs-polymarket-us.mintlify.app/api-reference/market/overview
- 10: https://docs.polymarket.com/api-reference/markets/list-markets
Avoid paginating on mutable liquidity. A market can move across the cursor between requests and be skipped entirely; the conditionId dedupe only removes repeats, not misses. Also, _fetch_price_history catches RequestException and returns None, so the backoff wrapper never retries and transient CLOB failures silently drop markets from the output.
|
tests pass (my old-new test harness) |
2 participants
closes #194
No clue actually why the
liquidity_num_minparam was not used for server-side filtering; it results in more consistent behaviour and 10x fewer requests; i've tested and validated this param and it works as expected; the5ff64b0commit adds this (still keeping the client-side filtering for robustness)fixes a dormant bug where the
/marketsendpoint can onlyoffsetup to 2500, while the qualifying markets are ~1000 more at the moment. Verified with:Summary by CodeRabbit