Skip to content

fix: oversubscription perps top movers#33150

Merged
gambinish merged 12 commits into
mainfrom
fix/oversubscription-perps-top-movers
Jul 14, 2026
Merged

fix: oversubscription perps top movers#33150
gambinish merged 12 commits into
mainfrom
fix/oversubscription-perps-top-movers

Conversation

@gambinish

@gambinish gambinish commented Jul 10, 2026

Copy link
Copy Markdown
Member

Description

Problem

The Now-tab perps movers strip subscribed to live prices for all ~329 markets and, on every ~3s push, rebuilt/re-sorted the full set and re-rendered — just to show the top 12 pills. Since Explore tabs never unmount, this kept running even on other tabs or after leaving Explore.

Why we keep the full-market subscription

Subscribing to only the 12 displayed symbols would break ranking: a market outside the top 12 can move into it, and we'd miss that price change. Correct live re-ordering requires watching every market.

The middle ground

The WebSocket already pushes all markets regardless of subscription — the real cost was the unconditional setState, the per-tick re-sort, and new object identities per tick. The new usePerpsLiveMovers hook merges every tick into a ref (no render per tick) and derives the displayed top-12 synchronously in render, so base-data loads and gainers/losers toggles reflect immediately with no empty frame. Live ticks only trigger a render when the displayed top-12 actually changes (via a symbol:formattedPercent fingerprint), reusing prior item references so unchanged pills skip re-render.

The full rank/filter/sort pass itself is also throttled to a configurable interval (recomputeIntervalMs, 10s on the Now tab): ticks arriving inside the interval accumulate losslessly in the ref, and at most one ranking pass runs per interval using the freshest data. Idle ticks cost only a cheap ref merge. On (re)subscribe — mount, tab resume, symbol-set change — the throttle anchor resets so the stream's cached snapshot surfaces promptly instead of waiting out a full interval.

On top of that, the subscription pauses entirely when not visible: an ExploreActiveTabContext tells PerpsBlock whether the Now tab is active, combined with useIsFocused() for leaving Explore. The same focus gating is applied to the What's Happening detail view's perps price hook. Disabling freezes the last-known data rather than clearing it, so nothing blanks out and it refreshes immediately on resume.

Changelog

CHANGELOG entry: improve performance on Perps Movers in explore

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

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
Touches live perps ranking, WebSocket subscription lifecycle, and Explore tab rendering; behavior is heavily tested but incorrect gating could show stale movers or miss updates.

Overview
Reduces Explore perps movers overhead by replacing per-tick usePerpsLivePrices + full re-sort in PerpsBlock with a new usePerpsLiveMovers hook that still watches all markets (so top-12 ranking stays correct) but merges ticks into a ref, only re-renders when the visible movers fingerprint changes, batches ranking with a 10s interval, and preserves pill object identity for unchanged symbols.

Pauses live work when Explore isn’t visible: ExploreActiveTabContext + an ExploreTabs subtree isolate tab state so switching tabs doesn’t re-render inactive tab feeds; PerpsBlock enables the movers subscription only when the screen is focused and the Now tab is active. What’s Happening detail pauses perps prices the same way by passing an empty symbol list while unfocused.

Tests cover the new context, tab-switch render behavior, live-movers hook edge cases, and subscription gating on Now tab.

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

@metamask-ci metamask-ci Bot added the team-perps Perps team label Jul 10, 2026
@metamask-ci

metamask-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Description section is empty. Describe what changed and why.
  • Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.
  • Manual testing steps still contain template content (the Gherkin example title or a [...] placeholder). Replace with real steps, or write N/A — <reason>.
  • Screenshots/Recordings section is empty. Add an image/video for user-facing changes, logs/console output for non-user-facing changes, or write N/A if no evidence is applicable.
  • Pre-merge author checklist has unchecked items (e.g. "I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards."). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@gambinish
gambinish marked this pull request as ready for review July 13, 2026 21:14
@gambinish
gambinish requested a review from a team as a code owner July 13, 2026 21:14
Comment thread app/components/Views/TrendingView/feeds/perps/usePerpsLiveMovers.ts Outdated
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jul 13, 2026
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/Views/TrendingView/tabs/NowTab.test.tsx 0/86 0/179 0/362

AI-detected flaky patterns

app/components/Views/TrendingView/tabs/NowTab.test.tsx

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() in beforeEach (high)
    • jest.clearAllMocks() is called inside the arrangeMocks() helper rather than in a beforeEach hook. Every describe block delegates to arrangeMocks() (via arrangePerpsMoversMocks(), arrangeCryptoMoversMocks(), etc.), so in the current test suite this works. However, this is a fragile pattern: any future test that is added without calling arrangeMocks() — or that calls it after some side-effectful setup — will inherit stale call counts and mock implementations from the previous test. Jest runs tests in file order by default, but --randomize or parallel sharding can expose the ordering dependency. The fix is to move jest.clearAllMocks() (and the default mock implementations) into a top-level beforeEach so isolation is guaranteed unconditionally.
    • Suggested fix in app/components/Views/TrendingView/tabs/NowTab.test.tsx:248:
      -const arrangeMocks = () => {
      -  jest.clearAllMocks();
      -  mockUseIsFocused.mockReturnValue(true);
      -
      -  const mockUseSelector = useSelector as jest.MockedFunction<
      -    typeof useSelector
      -  >;
      -  const mockUseTokensFeed = useTokensFeed as jest.MockedFunction<
      -    typeof useTokensFeed
      -  >;
      -
      -  mockUseSelector.mockImplementation(
      -    createMockSelectorImpl({
      -      perpsEnabled: false,
      -      predictEnabled: false,
      -      whatsHappeningEnabled: false,
      -    }),
      -  );
      -  // ... (rest of mock setup)
      -};
      +// Add a top-level beforeEach that always clears mocks and restores defaults.
      +// Keep arrangeMocks() for the per-describe overrides, but remove the
      +// jest.clearAllMocks() call from inside it.
      +
      +beforeEach(() => {
      +  jest.clearAllMocks();
      +  // Restore module-level mock defaults so every test starts from a known state.
      +  mockUseIsFocused.mockReturnValue(true);
      +  mockSubscribeToSymbols.mockReturnValue(jest.fn());
      +  mockWhatsHappeningRefresh.mockReset();
      +  mockUseWhatsHappening.mockReturnValue({
      +    items: [{ id: 'trend-0' }],
      +    isLoading: false,
      +    error: null,
      +    refresh: mockWhatsHappeningRefresh,
      +  });
      +});
      +
      +// Inside arrangeMocks(), remove the jest.clearAllMocks() line:
      +const arrangeMocks = () => {
      +  // jest.clearAllMocks(); ← remove this line
      +  const mockUseSelector = useSelector as jest.MockedFunction<typeof useSelector>;
      +  // ... rest of per-describe setup unchanged
      +};
  • J9 — Module-level mutable let bindings not reset in beforeEach (high)
    • All module-level mock functions (mockNavigate, mockUseIsFocused, mockSubscribeToSymbols, mockWhatsHappeningRefresh, mockUseWhatsHappening, mockNavigateToPerpsMarketList, mockPredictionsCarouselSection) are declared at module scope and their call history / implementations are only reset inside arrangeMocks(), which is called manually per test. If a test is skipped, fails early, or is added without calling arrangeMocks(), the accumulated call counts and overridden implementations from the previous test bleed into the next one. For example, mockUseIsFocused is set to false in one 'live movers subscription gating' test and is never explicitly restored to true unless arrangeMocks() is called next. Moving the reset to beforeEach eliminates this ordering dependency.
    • Suggested fix in app/components/Views/TrendingView/tabs/NowTab.test.tsx:5:
      -const mockNavigate = jest.fn();
      -const mockUseIsFocused = jest.fn(() => true);
      -
      -// ...
      -
      -const mockSubscribeToSymbols = jest.fn(() => jest.fn());
      -
      -// ...
      -
      -const mockWhatsHappeningRefresh = jest.fn();
      -const mockUseWhatsHappening = jest.fn(() => ({
      -  items: [] as { id: string }[],
      -  isLoading: false,
      -  error: null as string | null,
      -  refresh: mockWhatsHappeningRefresh,
      -}));
      -
      -// ...
      -
      -const mockNavigateToPerpsMarketList = jest.fn();
      -const mockPredictionsCarouselSection = jest.fn(/* ... */);
      +// At the top level of the describe block (or file), add:
      +beforeEach(() => {
      +  jest.clearAllMocks();
      +  // Re-apply default implementations for every module-level mock:
      +  mockUseIsFocused.mockReturnValue(true);
      +  mockSubscribeToSymbols.mockImplementation(() => jest.fn());
      +  mockUsePerpsFeed.mockReturnValue({
      +    data: [],
      +    isLoading: false,
      +    refetch: jest.fn(),
      +    defaultSortOptionId: 'priceChange' as const,
      +  });
      +  mockUseWhatsHappening.mockReturnValue({
      +    items: [],
      +    isLoading: false,
      +    error: null,
      +    refresh: mockWhatsHappeningRefresh,
      +  });
      +});

This check is informational only and does not block merging.

@codecov-commenter

codecov-commenter commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.53125% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.38%. Comparing base (9aad058) to head (daa855c).
⚠️ Report is 51 commits behind head on main.

Files with missing lines Patch % Lines
...ews/TrendingView/feeds/perps/usePerpsLiveMovers.ts 95.74% 0 Missing and 4 partials ⚠️
app/components/Views/TrendingView/TrendingView.tsx 85.71% 0 Missing and 2 partials ⚠️
app/components/Views/TrendingView/tabs/NowTab.tsx 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #33150      +/-   ##
==========================================
+ Coverage   84.31%   84.38%   +0.06%     
==========================================
  Files        6080     6129      +49     
  Lines      162378   163629    +1251     
  Branches    39577    39903     +326     
==========================================
+ Hits       136905   138074    +1169     
- Misses      16021    16046      +25     
- Partials     9452     9509      +57     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added size-XL and removed size-L labels Jul 13, 2026

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9f36d83. Configure here.

Comment thread app/components/Views/TrendingView/feeds/perps/usePerpsLiveMovers.ts
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 13, 2026

@geositta geositta left a comment

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.

Nice work overall addressing the root cause. Before approving, please fix the stale market data path. I also recommend narrowing the active tab update so this performance change does not rerender every loaded Explore feed.

Comment thread app/components/Views/TrendingView/feeds/perps/usePerpsLiveMovers.ts
Comment thread app/components/Views/TrendingView/TrendingView.tsx Outdated
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeWalletPlatform, SmokePerps, SmokePredictions, SmokeConfirmations
  • Selected Performance tags: @PerformancePreps
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR introduces performance optimizations to the TrendingView and related components:

  1. TrendingView.tsx: Refactored to extract ExploreTabs component with ExploreActiveTabProvider, memoizing tab elements to prevent re-renders on tab switches. This directly affects the Trending discovery tab tested by SmokeWalletPlatform.

  2. ExploreActiveTabContext.tsx (new): A React context exposing the active Explore tab to descendants, used to pause live subscriptions when a tab is not active.

  3. usePerpsLiveMovers.ts (new): A new hook for ranking perps markets by live 24h price-change percentage with throttling. This replaces the previous usePerpsLivePrices + manual filtering approach in NowTab.

  4. NowTab.tsx: Refactored to use usePerpsLiveMovers and adds useIsFocused/useExploreActiveTab to pause live subscriptions when the screen/tab isn't active. This is a behavioral change for the Perps section in the Now tab.

  5. useWhatsHappeningAssetPrices.ts: Adds useIsFocused to pause live perps price subscriptions when the WhatsHappeningDetailView is not focused — this affects the Predictions detail view.

Tag selection rationale:

  • SmokeWalletPlatform: TrendingView is the core Trending discovery tab; the tab switching logic and rendering behavior changed significantly.
  • SmokePerps: The Perps section within Trending (NowTab's PerpsBlock) was significantly refactored with new subscription management logic. Per tag description, SmokePerps also requires SmokeWalletPlatform (already selected).
  • SmokePredictions: useWhatsHappeningAssetPrices is used in WhatsHappeningDetailView which is part of the Predictions flow. Per tag description, SmokePredictions also requires SmokeWalletPlatform (already selected).
  • SmokeConfirmations: Required as a dependent tag when selecting both SmokePerps and SmokePredictions (per their tag descriptions, deposits/transactions go through confirmations).

Performance Test Selection:
The PR significantly refactors the Perps live movers subscription logic in the Trending/NowTab view. The new usePerpsLiveMovers hook introduces throttling and render-batching optimizations specifically designed to reduce re-renders during WebSocket price updates. This directly impacts the perpetuals trading performance scenario (@PerformancePreps), which covers perps market loading and the add funds flow. The changes could affect measured render times and subscription behavior in the perps section.

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 · 2 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

✅ Passed Tests (2)
Test Platform Device Duration Team Recording
Perps add funds Android Google Pixel 8 Pro (v14.0) 9.21s @mm-perps-engineering-team 📹 Watch
Perps open position and close it Android Google Pixel 8 Pro (v14.0) 20.84s @mm-perps-engineering-team 📹 Watch

Branch: fix/oversubscription-perps-top-movers · Build: Normal · Commit: d3a166c · View full run

@gambinish
gambinish enabled auto-merge July 14, 2026 22:28
@gambinish
gambinish added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit d29e494 Jul 14, 2026
190 of 191 checks passed
@gambinish
gambinish deleted the fix/oversubscription-perps-top-movers branch July 14, 2026 23:03
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 14, 2026
@metamask-ci metamask-ci Bot added the release-8.4.0 Issue or pull request that will be included in release 8.4.0 label Jul 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-8.4.0 Issue or pull request that will be included in release 8.4.0 risk:medium AI analysis: medium risk size-XL team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants