Acceptance Criteria Validation

Code Quality

• Pattern adherence: Follows codebase conventions — optional args with _ prefix for ignored params, JSDoc on the modified type, ?? fallback chain.

• Complexity: Minimal. Single-line logic change (positionFromRoute ?? currentPositionRef.current) at the call site. Signature propagation to three call sites is mechanical.

• Type safety: The return type of onConfirm was changed from Promise<void> to Promise<{ success: boolean } | void>. This makes the existing return { success: false } in the PerpsMarketDetailsView callback type-correct (it was previously returning an object from a Promise<void> function). All call sites compile cleanly — TSC passes with zero errors.

• Error handling: Adequate for the stated goal. The { success: false } return is consistent with the pattern used in handleUpdateTPSL.

• Anti-pattern findings:

  1. PerpsTPSLView.tsx:393 — Silent dismissal on double-null position (suggestion): If both positionFromRoute and currentPositionRef.current are null, onConfirm returns { success: false } but PerpsTPSLView.handleConfirm calls navigation.goBack() unconditionally. Because handleUpdateTPSL is never reached, no error toast is shown. The user sees the TPSL screen close with no feedback. This edge case is extremely unlikely (position was checked before navigation), but worth noting.

  2. PerpsMarketTabs.tsx:333 — No-op onConfirm (pre-existing, not introduced by PR): The PerpsMarketTabs callback remains a complete no-op. When TP/SL is confirmed from the market-tabs position tab, no API call is made. The comment attributes this to WebSocket-based position updates, but TP/SL persistence typically requires an explicit server call. This is pre-existing behavior — the PR only adds the required signature parameters. Not a blocker for this PR.

  3. types/navigation.ts:187 — Non-serializable navigation param (pre-existing): onConfirm is a function passed as a navigation param, which triggers React Navigation's Non-serializable values warning in Metro. This is a known pre-existing pattern in the codebase and not introduced by this PR.

Live Validation

• Recipe: generated (recipe.json)
• Result: PASS (15/15 steps)
  • check-positions: ✅ BTC and ETH positions present
  • nav-market-details: ✅ navigated to BTC market details
  • press-auto-close-toggle: ✅ opened TP/SL view
  • press-tp-10pct: ✅ +10% TP calculated and entered
  • watch-logs-before-confirm: ✅ no "No position found"
  • press-set-button: ✅ confirm dispatched
  • watch-logs-after-confirm: ✅ no "No position found"
  • All screenshots captured in evidence/
• Video: evidence/review.mp4 (2.6 MB, captured full recipe execution)
• Native changes: none
• Metro errors: none related to this PR. Pre-existing warnings: react-native-keychain server string, multichain require cycle, ActionModal defaultProps deprecation.
• Log monitoring: ~90s monitored during recipe execution; no errors introduced by this change.

Correctness

• Diff vs stated goal: Aligned. The diff does exactly what the PR description says.
• Edge cases:
  • Position null at confirm (both route param and ref null): handled by return { success: false } — but no user-visible error toast (see anti-pattern finding Inject inpage js #1).
  • Position updated via WebSocket between navigation and confirm: fixed — route param reflects state at navigation time; the fallback to ref covers unusual cases where param wasn't passed.
  • Order flow (no position): _position is ignored — correct, PerpsOrderView only needs TP/SL prices.
  • Market tabs flow: no-op callback unchanged — pre-existing behavior.
• Race conditions: The original race condition (stale ref between navigation and confirm) is eliminated by using route params. The new approach introduces a different tradeoff: the position at route-param time might differ from the confirm-time state if the position was significantly updated (e.g., partial close). The PR description acknowledges this is acceptable — it matches what the user saw when they intended to set TP/SL.
• Backward compatibility: The onConfirm signature change is internal to the Perps stack. The PerpsTPSL route type is updated and all three callers updated in the same PR. No external callers.

Static Analysis

• lint:tsc: PASS — 0 errors (incremental check with .tsbuildinfo)
• Tests: 84/84 pass (PerpsTPSLView.test.tsx + PerpsMarketDetailsView.test.tsx)

Architecture & Domain

The approach (pass position via nav params rather than relying on a ref) is the correct pattern for React Navigation flows. Refs are for in-component state across re-renders; cross-screen data should flow via navigation params. This PR aligns the pattern with React Navigation best practices.

The signature (position?, takeProfitPrice?, stopLossPrice?, trackingData?) is readable and correctly encodes the "position is optional from the caller's perspective" semantics. The ?? fallback ensures no regression for callers that don't pass position.

Risk Assessment

• LOW — Targeted fix. All changed code is in the UI layer. No controller, selector, or reducer changes. Type check passes. 84 tests pass. Live recipe validation confirms the fix on device.

Recommended Action

APPROVE

The fix is correct, minimal, and well-tested. The one suggestion worth raising is the silent-dismissal edge case (anti-pattern finding #1) — consider showing an error toast when positionToUse is null before returning { success: false }, so the user isn't left wondering why the screen closed without saving.

• [suggestion] app/components/UI/Perps/Views/PerpsTPSLView/PerpsTPSLView.tsx:393: Suggestion: navigation.goBack() is called unconditionally after onConfirm. If onConfirm returns { success: false } (which happens when both positionFromRoute and currentPositionRef.current are null), the TPSL screen closes with no user-visible error — handleUpdateTPSL was never reached so its internal toast never fires. The edge case is extremely unlikely in practice (position is verified before navigation), but consider adding a guard:

const result = await onConfirm(position, parseTakeProfitPrice, parseStopLossPrice, trackingData);
if (result && !result.success) {
  // show error toast here instead of navigating back
  return;
}
navigation.goBack();

This makes failures explicit to the user rather than silently closing the sheet.

• [nitpick] app/components/UI/Perps/components/PerpsMarketTabs/PerpsMarketTabs.tsx:339: Nitpick (pre-existing, not introduced by this PR): The onConfirm callback here is a complete no-op — TP/SL values are discarded. The comment says updates happen via WebSocket, but TP/SL persistence typically requires an explicit API call. If this flow is intentionally disabled, it may be worth tracking the pre-existing gap as a separate ticket to avoid user-facing silent no-ops when accessing TP/SL from the market tabs position view.

E2E Test Selection:
The changes are focused on the Perps TP/SL (Take Profit/Stop Loss) flow within the Perps feature:

1. navigation.ts: The onConfirm callback signature in PerpsNavigationParamList was updated to add a new first parameter position?: Position and change the return type to Promise<{ success: boolean } | void>. This is a breaking interface change that affects all callers.

2. PerpsTPSLView.tsx: Now passes position from route params as the first argument to onConfirm to fix a race condition where the parent's position ref could be stale.

3. PerpsMarketDetailsView.tsx: Updated to use positionFromRoute ?? currentPositionRef.current - prefers the position passed from TPSL view over the potentially stale ref.

4. PerpsOrderView.tsx and PerpsMarketTabs.tsx: Updated to accept the new _position parameter (ignored in these flows).

5. Test files: Unit tests updated to match new callback signatures.

Tag selection rationale:

• SmokePerps: Primary tag - these changes directly affect the Perps TP/SL flow (editing existing positions, setting take profit/stop loss). The bug fix changes how position data flows through the confirmation callback, which is a core Perps interaction.
• SmokeWalletPlatform: Required per SmokePerps tag description - "When selecting SmokePerps, also select SmokeWalletPlatform (Trending section)". Perps is also a section inside the Trending tab.
• SmokeConfirmations: Required per SmokePerps tag description - "When selecting SmokePerps, also select SmokeConfirmations (Add Funds deposits are on-chain transactions)". The TP/SL update flow involves on-chain transactions.

The risk is medium because while the changes are contained to Perps, the callback signature change touches multiple components and fixes a race condition that could affect position management reliability.

Performance Test Selection:
The changes touch the Perps TP/SL confirmation flow and position management. While the changes are primarily bug fixes (race condition fix for stale position ref), the callback signature change and position data flow modifications could potentially affect the performance of the Perps add funds and position management flows. Running @PerformancePreps is appropriate to ensure no performance regression was introduced in the Perps trading interface.