Skip to content

fix(predict): hide empty dynamic feed filters#33374

Closed
ghgoodreau wants to merge 1 commit into
mainfrom
pred-1085
Closed

fix(predict): hide empty dynamic feed filters#33374
ghgoodreau wants to merge 1 commit into
mainfrom
pred-1085

Conversation

@ghgoodreau

@ghgoodreau ghgoodreau commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Polymarket's omit_empty=true related-tags response only removes globally empty tags. Dynamic filter chips in the feature-flagged Predict home redesign could therefore resolve to zero visible markets after applying the feed parameters and client visibility rules.

This change validates each normalized dynamic filter with its exact market-list parameters and the same standalone-market and staleness filtering used by the redesigned feed. Filters with no visible markets are omitted while valid filters remain available. Regression coverage verifies both hidden-market filtering and preservation of the configured market page size.

Changelog

CHANGELOG entry: null

Related issues

Fixes: PRED-1085

Manual testing steps

Feature: Predict dynamic feed filters

  Scenario: User opens a redesigned Predict feed with an empty related tag
    Given the predictHomeRedesign feature flag is enabled
    And the related-tags response includes the "Other" filter
    And "Other" has no standalone visible markets for the applied feed parameters

    When the user opens the redesigned Predict feed
    Then the "Other" filter chip is hidden
    And dynamic filters with visible markets remain available

Screenshots/Recordings

N/A: This is a provider-level filtering change covered by automated tests; no screenshots or recordings were captured.

Before

N/A

After

N/A

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
    • N/A: This provider-level filtering change was verified with unit tests rather than a device build.
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
    • N/A: The behavior does not depend on wallet size, account count, or token count.
  • I've instrumented key operations with Sentry traces for production performance metrics
    • See trace() for usage and addToken for an example
    • N/A: Existing listFilterOptions and listMarkets controller operations are already traced; this change adds no new operation.

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Adds one listMarkets call per dynamic filter at load time (parallel), which increases API load and could slow filter discovery, but changes are confined to feed filter chips with no auth or funds impact.

Overview
Dynamic Polymarket related-tag filters can still appear in the redesigned Predict home even when no markets would show after feed params and client-side visibility rules—API omit_empty only drops globally empty tags.

PolymarketProvider.listFilterOptions now probes each normalized option with listMarkets(option.params), then keeps the chip only if getVisiblePredictMarkets(filterStandaloneMarkets(markets)) has at least one market (same pipeline as the feed/carousel hooks). Options that would render an empty list are dropped.

Tests add createVisibleMarket, stub listMarkets on existing filter-option cases, and cover hiding filters with only child/empty-outcome markets plus honoring baseParams.limit during validation.

Reviewed by Cursor Bugbot for commit b36370b. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamask-ci metamask-ci Bot added the team-predict Predict team label Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePredictions
  • Selected Performance tags: @PerformancePredict
  • Risk Level: medium
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are scoped to the PolymarketProvider's listFilterOptions method, which now validates filter options by checking if they have visible markets before returning them. This directly affects the Predictions feature (Polymarket integration) by filtering out empty filter categories in the UI.

The SmokePredictions tag is the appropriate selection because:

  1. The changes are in app/components/UI/Predict/providers/polymarket/PolymarketProvider.ts - directly in the Predict/Polymarket feature area
  2. The listFilterOptions method affects how filter options are displayed in the Predictions UI
  3. SmokePredictions tests cover the full position lifecycle and error handling scenarios for Polymarket

Per SmokePredictions tag description, when selecting SmokePredictions, also select SmokeWalletPlatform (Trending section) and SmokeConfirmations. However, the changes here are to the filter options logic (not to position opening/closing or transaction flows), so the risk to confirmations is minimal. Still, following the tag dependency guidance, SmokeWalletPlatform should be included since Predictions is a section inside the Trending tab.

The test file changes are unit tests only and don't affect E2E behavior directly, but the production code change in PolymarketProvider.ts warrants running the Predictions smoke tests to validate the filter option behavior works correctly end-to-end.

Performance Test Selection:
The PolymarketProvider.listFilterOptions method now makes additional async calls (Promise.all over listMarkets for each filter option) to validate market availability. This could impact performance of the Predictions feature by adding extra API calls during filter option loading. The @PerformancePredict tag covers prediction market list loading and balance display, which would be affected by this additional network overhead.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

All tests passed · 3 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

✅ Passed Tests (3)
Test Platform Device Duration Team Recording
Predict Market Details - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 3.83s @team-predict 📹 Watch
Predict Deposit - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 300.29s @team-predict
Predict Available Balance - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 300.21s @team-predict

Branch: pred-1085 · Build: Normal · Commit: ae5215f · View full run

@ghgoodreau ghgoodreau marked this pull request as ready for review July 15, 2026 19:36
@ghgoodreau ghgoodreau requested a review from a team as a code owner July 15, 2026 19:36
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jul 15, 2026
Comment on lines +1078 to +1086
const marketAvailability = await Promise.all(
filterOptions.map(async (option) => {
const { markets } = await this.listMarkets(option.params);
return (
getVisiblePredictMarkets(filterStandaloneMarkets(markets)).length >
0
);
}),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This affects performance, because every time we call this we're fetching a list of markets for every tag/chip. Without using data services, which allow us to share Tanstack Query cache between this layer and the UI layer, this becomes tricky, but we should revisit this change more carefully, as performance is of the utmost important.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, looking closer at the "related tags" API request to Polymarket, I noticed they actually have the number of active events for each tag in the response. So we should just use that and skip the extra API calls.

Screenshot 2026-07-16 at 8 25 34 AM

@matallui

Copy link
Copy Markdown
Contributor

Going to close this PR in favor of #33399

@matallui matallui closed this Jul 16, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

risk:low AI analysis: low risk size-M team-predict Predict team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants