Feat/prediction markets list#46
Conversation
- Add MinEndDate and MaxEndDate to MarketsFilter struct - Allows filtering markets by resolution window - Reduces result set to only relevant near-term markets - Prevents API from returning all 10k+ markets
This reverts commit 73ef4d3.
- Add FetchOrderBooksForMarket() to Connector interface for efficient batch loading - Implement batch orderbook fetching in polymarket connector using SDK's OrderBooks() API - Update Orderbook() method in predict SDK to lazy-load and cache full market's orderbooks - Add market loader that paginates markets and batch-fetches all orderbooks per market - Markets are fetched async in background, orderbooks cached in store for instant access
…nt slug
- Add Events []Event field to gamma Market struct to capture parent event data
- Add EventSlug field to prediction Market struct
- PolymarketURL() now uses /event/{event_slug} format which is canonical
- Fix Markets() to fetch synchronously when store is empty instead of silent goroutine
- Fix Event.Liquidity/Volume unmarshal: gamma API returns number not string
There was a problem hiding this comment.
Pull request overview
Adds prediction-market discovery support by introducing a market metadata store extension and async market loading, plus connector/API surface expansions to support pagination and batch orderbook retrieval.
Changes:
- Introduce
MarketStoreExtension+ in-memory implementation to cache markets per exchange. - Extend
Predictwith cachedMarkets(), explicitRefreshMarkets(), and backgroundLoadMarkets()with progress reporting. - Expand prediction connector interfaces/models (pagination filters, batch orderbook fetch, position split/merge helpers, Polymarket URL fields).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/markets/prediction/types/store_prediction.go | Extends prediction MarketStore to include market-metadata extension. |
| pkg/markets/prediction/types/store_market.go | Defines market metadata storage interface + MarketMap type. |
| pkg/markets/prediction/types/predict_sdk.go | Adds cached/refresh/background market-loading APIs to the Predict interface. |
| pkg/markets/prediction/types/market_loader.go | Introduces MarketLoader interface for async pagination + progress. |
| pkg/markets/prediction/types/connector/market.go | Adds tags/event slug fields and a Polymarket URL helper on Market. |
| pkg/markets/prediction/types/connector/connector.go | Expands MarketsFilter and connector capabilities (batch orderbooks, split/merge). |
| pkg/markets/prediction/store/store.go | Wires new market-metadata store extension into the prediction store. |
| pkg/markets/prediction/store/extensions/market.go | Implements in-memory market metadata cache keyed by exchange/marketID. |
| pkg/markets/prediction/predict/predict.go | Implements caching, refresh, background load delegation, and batch orderbook fallback. |
| pkg/markets/prediction/module.go | Registers NewMarketLoader for fx DI. |
| pkg/markets/prediction/market_loader.go | Implements background pagination, store updates, and orderbook prefetch. |
| pkg/config/settings/connectors/service.go | Coerces numeric credential strings to numbers before JSON unmarshal into SDK config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Create a copy of the filter for this batch | ||
| batchFilter := *filter |
There was a problem hiding this comment.
paginateAndLoad dereferences filter without checking for nil (batchFilter := *filter). The Connector.Markets contract allows nil filters, so calling LoadMarkets(..., nil) will panic. Handle nil by initializing a zero MarketsFilter for batching (or pass nil and set Limit/Offset on a local struct).
| batchFilter := *filter | |
| batchFilter := predictionconnector.MarketsFilter{} | |
| if filter != nil { | |
| batchFilter = *filter | |
| } |
| if len(storedMarkets) > 0 { | ||
| return storedMarkets, nil | ||
| } | ||
|
|
||
| // Store is empty - fetch synchronously so the caller gets actual results |
There was a problem hiding this comment.
Markets() returns cached markets from the store without applying the provided filter. If the store was populated by a previous call (potentially with a different filter), callers can receive results that violate the requested filters. Either apply the filter to cached data, or only serve from cache when filter == nil (and otherwise call RefreshMarkets).
| if len(storedMarkets) > 0 { | |
| return storedMarkets, nil | |
| } | |
| // Store is empty - fetch synchronously so the caller gets actual results | |
| if filter == nil && len(storedMarkets) > 0 { | |
| return storedMarkets, nil | |
| } | |
| // Store is empty, or a filtered result was requested - fetch synchronously so the caller gets actual results |
|
|
||
| if exMap, ok := e.markets[exchange]; ok { | ||
| if market, ok := exMap[marketID]; ok { | ||
| return &market |
There was a problem hiding this comment.
GetMarket returns a *predictionconnector.Market by taking the address of a local copy (market := exMap[marketID]; return &market). This pointer does not reference the value stored in the map, so any caller mutation won’t be reflected in the store and the pointer semantics are misleading. Consider returning (predictionconnector.Market, bool) instead, or store *predictionconnector.Market in the map and return the stored pointer.
| return &market | |
| marketCopy := new(predictionconnector.Market) | |
| *marketCopy = market | |
| return marketCopy |
| book = p.store.GetOrderBook(exchange, market.MarketID, outcome.OutcomeID) | ||
| if book == nil { | ||
| return nil, errors.New("order book not found for outcome: " + string(outcome.OutcomeID) + " on exchange: " + string(exchange)) | ||
| return nil, errors.New("order book not found for outcome: " + string(outcome.OutcomeID)) |
There was a problem hiding this comment.
The updated error message drops the exchange/market context that was previously included. Keeping exchange and market/outcome identifiers in the error improves diagnosability when multiple exchanges/markets are in use.
| return nil, errors.New("order book not found for outcome: " + string(outcome.OutcomeID)) | |
| return nil, errors.New("order book not found for exchange: " + string(exchange) + " market: " + string(market.MarketID) + " outcome: " + string(outcome.OutcomeID)) |
| "github.com/wisp-trading/sdk/pkg/types/connector" | ||
| ) | ||
|
|
||
| // MarketMap is Exchange -> Market |
There was a problem hiding this comment.
Comment says MarketMap is Exchange -> Market, but the type is ExchangeName -> MarketID -> Market. Update the comment to reflect the nested mapping to avoid confusion for callers.
| // MarketMap is Exchange -> Market | |
| // MarketMap is Exchange -> MarketID -> Market |
| func (ml *marketLoader) LoadMarkets( | ||
| exchange connector.ExchangeName, | ||
| filter *predictionconnector.MarketsFilter, | ||
| ) error { | ||
| ml.mu.Lock() | ||
| defer ml.mu.Unlock() | ||
|
|
||
| // Already loading for this exchange | ||
| if ml.loadingState[exchange] { | ||
| return fmt.Errorf("market loading already in progress for %s", exchange) | ||
| } | ||
|
|
||
| // Get connector | ||
| conn, exists := ml.connectorRegistry.Prediction(exchange) | ||
| if !exists { | ||
| return fmt.Errorf("connector not found for exchange: %s", exchange) | ||
| } | ||
|
|
||
| // Mark as loading | ||
| ml.loadingState[exchange] = true | ||
| ml.loadProgress[exchange] = 0 | ||
|
|
||
| // Create cancellable context | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| ml.activeCancelFuncs[exchange] = cancel | ||
|
|
||
| // Start background pagination loop | ||
| go ml.paginateAndLoad(ctx, exchange, conn, filter) | ||
|
|
||
| ml.logger.Info("Started background market loading for %s", exchange) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
New background market loading behavior (pagination, progress tracking, orderbook prefetch) is introduced here without tests. Adding unit tests for at least: nil filter handling, pagination termination (len < batchSize), and progress/state reset on error/cancel would help prevent regressions.
No description provided.