feat: add market filtering to Markets() discovery#45
Conversation
lwtsn
commented
Apr 9, 2026
lwtsn
commented
- Add MarketsFilter struct to filter by MinVolume and MinLiquidity
- Update Connector.Markets() signature to accept optional filter
- Update Predict.Markets() to pass filters to connector
- Allows SDK to request filtered results from API, reducing network load
- API applies filters server-side instead of client-side filtering
- Enables screener to discover only high-volume, liquid markets
- Add MarketsFilter struct to filter by MinVolume and MinLiquidity - Update Connector.Markets() signature to accept optional filter - Update Predict.Markets() to pass filters to connector - Allows SDK to request filtered results from API, reducing network load - API applies filters server-side instead of client-side filtering - Enables screener to discover only high-volume, liquid markets
There was a problem hiding this comment.
Pull request overview
This PR adds server-side market filtering to prediction market discovery by introducing a MarketsFilter and updating the prediction SDK surface to request filtered market lists from connectors (reducing client-side filtering and network load).
Changes:
- Added
MarketsFilter(min volume, min liquidity, active-only flag) to prediction connector types. - Updated
Predict.Marketsand prediction connectorConnector.Marketsto accept an optional filter and return([]Market, error). - Implemented
predict.Markets(...)to resolve the exchange connector and forward filters.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/markets/prediction/types/predict_sdk.go | Updates Predict interface Markets signature to include exchange + optional filter and return an error. |
| pkg/markets/prediction/types/connector/connector.go | Introduces MarketsFilter and adds Markets(filter *MarketsFilter) to the prediction connector interface. |
| pkg/markets/prediction/predict/predict.go | Implements Markets(exchange, filter) by dispatching to the exchange connector and returning errors on failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WatchMarket(exchange connector.ExchangeName, market predictionconnector.Market) | ||
|
|
||
| Markets() []predictionconnector.Market | ||
| Markets(exchange connector.ExchangeName, filter *predictionconnector.MarketsFilter) ([]predictionconnector.Market, error) |
There was a problem hiding this comment.
The mockery-generated mocks for types.Predict are now out of sync: mocks/.../pkg/markets/prediction/types/Predict.go still defines Markets() []Market, but the interface now requires Markets(exchange, filter) ([]Market, error). Regenerate/update the mocks so tests (and downstream users of the mocks) can compile against the new interface.
| // MarketsFilter specifies criteria for filtering markets returned by Markets(). | ||
| type MarketsFilter struct { | ||
| // MinVolume filters markets by minimum 24h volume (e.g., "1000.00") | ||
| MinVolume string | ||
| // MinLiquidity filters markets by minimum liquidity (e.g., "100.00") | ||
| MinLiquidity string | ||
| // Active filters to only active markets (default: true) | ||
| Active *bool | ||
| } |
There was a problem hiding this comment.
MarketsFilter docs are ambiguous about the default "active" behavior: the struct says Active defaults to true, while the Connector.Markets comment only mentions "Pass nil filters to get all active markets". Please clarify whether filter == nil and/or filter.Active == nil imply active-only, and how callers request all markets (including inactive) if that’s supported.
| // Markets returns markets available on this exchange, optionally filtered. | ||
| // Pass nil filters to get all active markets. | ||
| Markets(filter *MarketsFilter) ([]Market, error) |
There was a problem hiding this comment.
The mockery-generated prediction connector mocks appear out of date: mocks/.../pkg/markets/prediction/types/connector/Connector.go does not include the new Markets(filter *MarketsFilter) ([]Market, error) method. Regenerate/update these mocks to match the updated interface.
| markets, err := marketConnector.Markets(filter) | ||
|
|
||
| if err != nil { | ||
| return nil, errors.New("failed to fetch markets from exchange: " + string(exchange) + " error: " + err.Error()) | ||
| } |
There was a problem hiding this comment.
Error handling here drops the original error (string-concatenates err.Error()), which prevents callers from unwrapping/inspecting the underlying failure. Prefer fmt.Errorf with %w (and avoid manual string concatenation) to preserve the cause while still adding exchange context.
