fix(social): distinguish opened/added/reduced/closed trades (TSA-988)

[xavier-brochard]

Description

A trader's position moves through four events: they open it, add to it, reduce it, and close it. The feed and the trade list only knew intent: 'enter' | 'exit', so a partial exit announced "closed" — a follower saw "alice closed ETH" when alice had trimmed 10%, and the row's right column flipped to realized P&L on the strength of that same wrong boolean. The social-api's push copy already classified all four stages, so the three surfaces also disagreed with each other about the same fill.

resolveTradeActions (new, app/components/Views/SocialLeaderboard/utils/tradeAction.ts) derives the stage for every fill in a position. It prefers the API's new per-trade action, computed server-side against the full history, and otherwise walks the trades locally — so this ships without waiting on the social-api deploy or the @metamask/social-controllers bump, and silently upgrades once they land.

Three details worth a reviewer's attention:

1. Fills are signed by intent, never tokenAmount or direction. Opening a short is direction: 'sell' with intent: 'enter' while the size is reported as a positive magnitude, so signing by intent makes longs and shorts accumulate identically and one walk covers both.

2. The spot walk is anchored to positionAmount. The API caps a position at 50 fills and preferentially retains commented ones, so trades can start mid-position — accumulating from zero would report fill Introduce e2e tests via Detox #51 of a long-lived position as the one that opened it. Seeding with whatever positionAmount cannot account for recovers the truncated prefix, with a relative tolerance so ordinary reconciliation drift doesn't fabricate one.

3. Perps opt out of that anchor. They keep their historical positionAmount after closing — a closed 5x long still reports a size of 5, as perp.ts already documented — so seeding from it would invent a pre-window balance and read every closing perp fill as a reduce. Both paths defer to isClosedPosition for the newest exit, which now prefers the API's isOpen over its own heuristics.

Wording. Perps surface all four stages (Opened / Added / Reduced / Closed); spot surfaces only the direction (Bought / Sold). A token row carries no LONG/SHORT badge, and in the feed the figure beside the verb is P&L rather than trade direction, so for spot the verb is the only buy/sell signal the row has. Spot still carries the full four-state action underneath, because that is what decides whether a row realizes P&L — which is the actual bug fix.

Behaviour change to note. mapFeedItem previously used intent === 'exit' as a deliberate override for perps carrying stale non-zero margin. Removing it is the point of the ticket, but it means that without the API's isOpen, a full close on a stale-margin payload now reads "reduced" instead of "closed". Routine partial reduces were being mislabelled before; a stale-margin close is the rarer case, and it becomes exact once the API field ships.

Analytics. feed_action values change from bought/sold/opened/closed to opened/added/reduced/closed. Asset class is already carried by trade_type, but downstream dashboards will need updating.

Changelog

CHANGELOG entry: Fixed the trader feed and position views describing a partly reduced position as closed, and added distinct labels for adding to and reducing a position

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/TSA-988

Depends on (both additive, neither blocking this PR):

• feat(social-controllers): add the position-lifecycle stage of a trade core#9871 — @metamask/social-controllers types for Trade['action'] and Position['isOpen']
• consensys-vertical-apps/va-mmcx-social-api#210 — computes and exposes the fields, and aligns the push copy

Manual testing steps

Feature: Trade lifecycle wording in the social surfaces

  Scenario: a trader partly reduces a perp position
    Given the user follows a trader with an open perp position
    And that trader has sold part of it without closing it

    When user opens the Feed tab under Top traders
    Then the row reads "<trader> reduced" rather than "<trader> closed"
    And the right column still shows current value, not realized P&L

  Scenario: a trader adds to a perp position
    Given a trader whose position has more than one entry fill

    When user taps the position to open the trader position view
    Then the first fill reads "Opened" and later entry fills read "Added"
    And exit fills read "Reduced" until the one that empties the position, which reads "Closed"

  Scenario: spot rows name the direction
    Given a trader with a token position built from several buys and sells

    When user opens the trader position view for that token
    Then every buy reads "Bought" and every sell reads "Sold"

  Scenario: the feed and the position view agree
    Given any trade visible in the Feed tab

    When user taps through to the trader position view
    Then the verb shown for that fill matches the one the feed row used

Screenshots/Recordings

Before

Perp position view listed every entry as Opened and every exit as Closed, with no way to tell a trim from a full exit.

After

Pre-merge author checklist

• I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I've included tests if applicable
• I've documented my code using JSDoc format if applicable
• I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Performance checks (if applicable)

• I've tested on Android
• I've tested with a power user scenario
• I've instrumented key operations with Sentry traces for production performance metrics

No new rendering work: the action array is resolved once per position in useTraderPositionData / mapFeedItem, both already memoized, and each row receives its verb as a prop rather than deriving it.

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.

🤖 Generated with Claude Code

[github-actions]

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]

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

Warnings — informational, address before merging:

• Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). 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.

[github-actions]

🧪 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/SocialLeaderboard/FeedView/components/FeedItemRow.test.tsx	0/109	0/160	0/354

AI-detected flaky patterns

app/components/Views/SocialLeaderboard/FeedView/components/FeedItemRow.test.tsx

• J7 — Non-deterministic data: Date.now() (medium)
  • The spotItem and perpItem module-level fixtures use Date.now() to compute their timestamp fields (Date.now() - 21_000 and Date.now() - 4 * 60_000). These values are evaluated once at module-load time and baked into the fixture objects, so they are non-deterministic across CI runs.

No current test asserts on the rendered timestamp string directly, which is why this hasn't caused failures yet. However, the timestamp is passed to the component and may drive relative-time display logic (e.g. '21 seconds ago', '4 minutes ago'). If the component renders a human-readable age string and a test ever asserts on it — or if CI load causes the module to load at a different wall-clock offset — the assertion will fail intermittently.

Fix: replace both Date.now() expressions with a fixed Unix-millisecond constant so the fixture is fully deterministic regardless of when the test suite runs.

• Suggested fix in app/components/Views/SocialLeaderboard/FeedView/components/FeedItemRow.test.tsx:43:

  -const spotItem: FeedSpotItem = {
  -  id: 'spot-1',
  -  type: 'spot',
  -  traderId: 'trader-spot-1',
  -  username: 'dutchiono',
  -  traderAddress: '0x1111111111111111111111111111111111111111',
  -  action: 'opened',
  -  timestamp: Date.now() - 21_000,
  -  tokenSymbol: 'PEPE',
  +const FIXED_NOW_MS = 1_722_470_400_000; // 2024-08-01T12:00:00.000Z — pinned, deterministic
  +
  +const spotItem: FeedSpotItem = {
  +  id: 'spot-1',
  +  type: 'spot',
  +  traderId: 'trader-spot-1',
  +  username: 'dutchiono',
  +  traderAddress: '0x1111111111111111111111111111111111111111',
  +  action: 'opened',
  +  timestamp: FIXED_NOW_MS - 21_000,
  +  tokenSymbol: 'PEPE',

• J7 — Non-deterministic data: Date.now() (medium)
  • Same J7 issue as spotItem: perpItem.timestamp is set to Date.now() - 4 * 60_000 at module-load time. This is a live-clock value that changes on every test run. If the component renders a relative-time label (e.g. '4 minutes ago') and a test asserts on that text, the assertion will race the wall clock and fail intermittently under CI load.

Fix: use the same pinned constant (FIXED_NOW_MS) introduced for spotItem so both fixtures share a stable, deterministic reference point.

• Suggested fix in app/components/Views/SocialLeaderboard/FeedView/components/FeedItemRow.test.tsx:72:

  -const perpItem: FeedPerpItem = {
  -  id: 'perp-1',
  -  type: 'perps',
  -  traderId: 'trader-perp-1',
  -  username: 'aparjey',
  -  traderAddress: '0x2222222222222222222222222222222222222222',
  -  action: 'closed',
  -  timestamp: Date.now() - 4 * 60_000,
  +const perpItem: FeedPerpItem = {
  +  id: 'perp-1',
  +  type: 'perps',
  +  traderId: 'trader-perp-1',
  +  username: 'aparjey',
  +  traderAddress: '0x2222222222222222222222222222222222222222',
  +  action: 'closed',
  +  timestamp: FIXED_NOW_MS - 4 * 60_000,

This check is informational only and does not block merging.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud