High-priority issues

H1. Multi-provider race on _providerInSheetMode / _providerMounted globals

PredictPreviewSheetProvider is mounted in two places:

• app/components/UI/Predict/routes/index.tsx (PredictScreenStack, default → sheet mode)
• app/components/Views/Homepage/components/HomepageDiscoveryTabs/HomepageDiscoveryTabs.tsx (now disableBottomSheet)

When the user is on the Predictions tab inside HomepageDiscoveryTabs and taps a market, openBuySheet calls navigation.navigate(Routes.PREDICT.ROOT, { screen: BUY_PREVIEW, params }). This pushes onto the root navigator and mounts PredictScreenStack, which mounts a second PredictPreviewSheetProvider. For a window of time both providers are mounted, but the singleton booleans:

let _providerMounted = false;

/**
 * Returns whether `PredictPreviewSheetProvider` is currently mounted somewhere
 ...
 */
export function isPredictSheetProviderMounted(): boolean {
  return _providerMounted;
}

…cannot represent two providers simultaneously. The mount/unmount effects race, so:

• After the inner provider unmounts (e.g., user pops BUY_PREVIEW), it sets _providerMounted = false and _providerInSheetMode = false, even though the outer (disableBottomSheet) provider is still mounted.
• After the inner provider mounts, it sets _providerInSheetMode = true, even though the outer one is disableBottomSheet.

In today's code this happens to produce the correct toast behavior in the most common flow because the outer provider's retry-toast effect short-circuits on disableBottomSheet. But the design is fragile, any future consumer of these globals (or any reordering of useEffect runs in StrictMode/dev) can flip behavior silently. It's also genuinely incorrect: there is a Predict provider mounted, and during the interval the inner stack is mounted the outer one is implicitly "in sheet mode" from the global's perspective, even though it explicitly opted out.

Suggested fix: replace the two booleans with reference counters, e.g.:

let _providerMountedCount = 0;
let _providerSheetModeMountedCount = 0;
export function isPredictSheetProviderMounted() {
  return _providerSheetModeMountedCount > 0;
}

…incremented on mount and decremented on unmount based on disableBottomSheet. This makes the behavior independent of mount/unmount ordering when multiple providers coexist.

H2. Analytics behavior change in PredictBuyPreview screen mode is broader than the stated scope

The PR description says the beforeRemove listener exists for screen mode in the new disableBottomSheet flow. The implementation, however, registers the listener for every screen-mode mount of PredictBuyPreview:

useEffect(() => {
  if (isSheetMode) return;
  return addListener('beforeRemove', () => {
    if (!predictBuyPreviewOrderInitiatedRef.current) {
      const dismissalMethod = predictBuyPreviewDismissedViaBackRef.current
        ? PredictDismissalMethod.BACK_BUTTON
        : PredictDismissalMethod.SWIPE;
      Engine.context.PredictController.trackBetslipDismissed({ ... });
    }
  });
}, [addListener, isSheetMode, analyticsProperties, transactionActiveAbTests]);

That includes the pre-existing path where bottomSheetEnabled === false (LD flag off) i.e., users who were already navigating to the full-screen buy preview before this PR. Previously those users only fired Predict Betslip Dismissed when they tapped the in-screen back button; swipe/hardware-back dismissals were silent. After this PR, those swipe/hardware-back dismissals will start firing SWIPE events.

This may well be desirable (it's arguably a correctness improvement and matches what usePredictBuyActions.ts already does for the AnyToken path), but the PR title and description frame it as a fix for the Hub Page Discovery Tabs A/B test only. Reviewers/analytics owners may be missing that the change ships analytics for all flagless users on this path. Two options:

• Gate the listener on props.mode === 'screen' && /* something distinguishing the new flow */ so the analytics change is scoped to the new flow, or
• Keep the broader change but call it out explicitly in the changelog/Jira and confirm with the analytics team that the new event volume is expected.

Medium-priority issues

M1. isPredictSheetProviderMounted() no longer matches its name

The function is documented as "whether PredictPreviewSheetProvider is currently mounted somewhere in the tree", but after this PR it returns _providerMounted && _providerInSheetMode:

let _providerMounted = false;
let _providerInSheetMode = false;

export function isPredictSheetProviderMounted(): boolean {
  return _providerMounted && _providerInSheetMode;
}

A disableBottomSheet-mounted provider is still mounted, but the function now lies about that. The only caller (usePredictToastRegistrations) actually uses this boolean to mean "will the provider's retry toast cover this failure?", not "is the provider mounted?". Rename to reflect the new semantics — e.g. shouldSuppressLegacyOrderFailureToast() or isPredictSheetProviderInSheetMode() — and update the doc comment. This avoids the next caller misinterpreting the function and (per H1) re-introducing the silent-failure bug that motivated this fix.

M2. Misleading inline comment in the back-button handler

        onPress={() => {
          predictBuyPreviewDismissedViaBackRef.current = true;
          if (isSheetMode) {
            // Sheet dismissals are tracked in PredictPreviewSheetContext.onBuyDismiss.
            Engine.context.PredictController.trackBetslipDismissed({
              ...
            });
            onClose?.();
          } else {
            // Screen mode: beforeRemove listener owns dismiss tracking.
            goBack();
          }
        }}

The first comment ("Sheet dismissals are tracked in PredictPreviewSheetContext.onBuyDismiss") sits directly above a block that does call trackBetslipDismissed. The intent is "swipe/hardware-back sheet dismissals are tracked elsewhere; this branch handles only the BACK_BUTTON sheet path", but as written it reads like a contradiction. Reword to something like:

// Sheet mode: this branch handles the explicit BACK_BUTTON dismissal.
// Swipe / hardware-back dismissals are handled by onBuyDismiss in PredictPreviewSheetContext.

M3. lastBuyParamsRef.current = params is a dead store under disableBottomSheet

const openBuySheet = useCallback(
  (params: PredictBuyPreviewParams) => {
    lastBuyParamsRef.current = params;          // ← always set
    if (bottomSheetEnabled && !disableBottomSheet) {
      ...
    } else {
      navigation.navigate(Routes.PREDICT.ROOT, {
        screen: Routes.PREDICT.MODALS.BUY_PREVIEW,
        params,
      });
    }
  },
  ...,
);

The retry-toast effect that consumes lastBuyParamsRef.current short-circuits on disableBottomSheet, so when disableBottomSheet === true we are storing params we will never read. Two minor concerns:

• We hold a long-lived strong reference to params.market, outcome, outcomeToken, etc., for the lifetime of the provider, even though we'll never use them.
• It muddies the retry contract: a future maintainer could be tempted to read lastBuyParamsRef from a code path that doesn't gate on disableBottomSheet and re-introduce the same silent-failure bug.

Either gate the assignment on !disableBottomSheet, or comment the intentional dead-store.

M4. Missing test: feature-flag-off + disableBottomSheet interaction

The new tests cover disableBottomSheet=true while bottomSheetEnabled is true, and the existing tests cover bottomSheetEnabled=false with no disableBottomSheet. There's no test for bottomSheetEnabled=false with disableBottomSheet=true. In that combo, both branches force navigation, but _providerInSheetMode is now driven purely by disableBottomSheet (i.e. false), regardless of the runtime feature flag — which is what we want, but should be locked in with a test so a refactor of the flag selectors can't regress it.

M5. JSDoc on shared module-level refs is now stale

/**
 * Module-level ref so PredictPreviewSheetContext can distinguish a programmatic
 * back-button dismiss from a swipe/hardware-back dismiss when onBuyDismiss fires.
 * Set to true in the back-button handler, reset to false by the sheet context
 * after consuming it.
 */
export const predictBuyPreviewDismissedViaBackRef = { current: false };

The doc now lies on three counts after this PR:

1. The ref is also consumed by the new beforeRemove listener in screen mode (PredictBuyPreview.tsx), not just by onBuyDismiss.
2. It is also consumed by usePredictBuyActions.ts for the AnyToken screen-mode path.
3. It is now reset on mount by PredictBuyPreview itself (line 121), in addition to being reset by onBuyDismiss.

Update the JSDoc to reflect that this is shared module-level state with three consumers, and document the reset contract to reduce future foot-guns.

High-priority issues

H1. hadEnteredAmount analytics value is now inconsistent across dismissal paths

Five distinct dismissal paths now emit Predict Betslip Dismissed, but they read hadEnteredAmount from two different sources:

The session ref is "sticky": once currentValue > 0 it flips to true for the rest of the session and is never set back to false (only reset on mount/unmount):

useEffect(() => {
  if (currentValue > 0) {
    predictBuyPreviewSessionRef.hadEnteredAmount = true;
  }
  ...
}, [currentValue, isCalculating]);

So a user who types $10, clears it back to $0, then taps back will report:

• hadEnteredAmount = false in sheet-mode back-button and screen-mode flagless back-button
• hadEnteredAmount = true in the new screen-mode-trackSwipeDismiss back-button (which defers to beforeRemove)

The same physical action (tap back-button) reports different analytics depending on which presentation mode the user happens to be in. This will skew the had_entered_amount distribution for the HomepageDiscoveryTabs cohort relative to the rest of Predict and contaminate the A/B test signal the PR is meant to support.

Suggested fix: pick one semantic and apply it everywhere. The sticky semantic is arguably the more useful one ("did the user ever engage?"), but whichever is chosen, all five paths should agree. At minimum, the new beforeRemove listener and the back-button handler should read from the same source when the user is in screen mode.

H2. beforeRemove listener fires on every pop, not just dismissals

useEffect(() => {
  if (isSheetMode || !trackSwipeDismiss) return;
  return addListener('beforeRemove', () => {
    if (!predictBuyPreviewOrderInitiatedRef.current) {
      const dismissalMethod = predictBuyPreviewDismissedViaBackRef.current
        ? PredictDismissalMethod.BACK_BUTTON
        : PredictDismissalMethod.SWIPE;
      Engine.context.PredictController.trackBetslipDismissed({ ... });
    }
  });
}, [addListener, isSheetMode, trackSwipeDismiss, ...]);

beforeRemove fires for any removal of the screen from the navigation stack, including programmatic StackActions.pop() triggered by a successful order:

useEffect(() => {
  if (result?.success) {
    if (isSheetMode) {
      onClose?.();
    } else {
      dispatch(StackActions.pop());   // ← will trigger beforeRemove
    }
  }
}, [dispatch, result, isSheetMode, onClose]);

The !predictBuyPreviewOrderInitiatedRef.current gate inside the listener is what's supposed to prevent the false-positive dismissal event in this flow. That works as long as onPlaceBet always runs first:

const onPlaceBet = useCallback(async () => {
  if (!preview || isBelowMinimum || isInsufficientBalance) return;
  predictBuyPreviewOrderInitiatedRef.current = true;   // ← gate setter
  await placeOrder({ analyticsProperties, preview });
}, ...);

But the gate is fragile in a few ways:

1. The ref is module-level and shared with PredictBuyWithAnyToken / usePredictBuyActions. If a previous PredictBuyWithAnyToken mount left the ref true and a PredictBuyPreview mounts in screen-mode-trackSwipeDismiss immediately after, the beforeRemove listener will silently swallow the first dismissal until the mount-time reset on line 125 lands. The mount-time reset does run, so the dominant ordering is safe — but the cross-component coupling is hard to verify by reading the code.
2. There's no test that asserts "successful order does not emit a SWIPE dismissal event". The test that comes closest only checks the negative case for predictBuyPreviewOrderInitiatedRef.current = true (line 2599) by manually toggling the ref. A higher-fidelity test would render the component, drive placeOrder to success, and assert trackBetslipDismissed was not called.

Suggested fix: add the integration-level test described above. Optionally, replace predictBuyPreviewOrderInitiatedRef with a stronger signal (e.g. checking result?.success or the e.data?.action.type === 'POP' style guard inside the listener).

H3. Screen-mode flagless BACK_BUTTON tracking is now gated on orderInitiatedRef — quietly different from previous behavior

Before this PR, the screen-mode back-button handler fired BACK_BUTTON unconditionally:

- // (previous main):
- predictBuyPreviewDismissedViaBackRef.current = true;
- Engine.context.PredictController.trackBetslipDismissed({
-   ...,
-   dismissalMethod: PredictDismissalMethod.BACK_BUTTON,
- });
- if (isSheetMode) onClose?.();
- else goBack();

After 8e24485, the flagless path now gates the event on !predictBuyPreviewOrderInitiatedRef.current:

} else {
  // Screen mode (flagless path): beforeRemove listener is not
  // registered, so track back-button dismissal directly here.
  if (!predictBuyPreviewOrderInitiatedRef.current) {
    Engine.context.PredictController.trackBetslipDismissed({
      analyticsProperties,
      dismissalMethod: PredictDismissalMethod.BACK_BUTTON,
      hadEnteredAmount: currentValue > 0,
      timeOnScreenMs: Date.now() - mountTimestampRef.current,
      activeAbTests: transactionActiveAbTests,
    });
  }
  goBack();
}

This is a behavior change for all existing users of the flagless screen-mode path (i.e. users who already see the full-screen preview because selectPredictBottomSheetEnabledFlag is false):

• Before: tap Place Bet → bet in-flight → tap back-button → BACK_BUTTON event fires.
• After: same sequence → event suppressed.

It's plausible that this is the desired semantic (the user committed; calling that a "dismissal" is dubious), but:

1. The sheet-mode back-button handler (line 381) was not updated symmetrically — it still fires BACK_BUTTON unconditionally even when orderInitiatedRef === true. So we now have:
   • Sheet-mode back-button after Place Bet: fires BACK_BUTTON event
   • Screen-mode back-button after Place Bet: silently swallows the event
2. The PR description doesn't call out this analytics change for the flagless cohort.
3. There is no test for this branch — the new describe('beforeRemove dismiss tracking (screen mode)', ...) block (line 2549) covers the trackSwipeDismiss=true cases, but not the new gated flagless back-button path.

Suggested fix: either revert the gate on the flagless back-button (keep prior unconditional firing), or apply the same gate to the sheet-mode back-button so all three back-button paths are consistent. Either way, add a test that locks in the chosen semantic for the flagless path, and mention the change in the PR body so the analytics team isn't surprised.

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

^{❌ 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 25b2bb1. Configure here.}

LGTM