Skip to content

Commit 96c4022

Browse files
paullinatorpaullinator
committed
Add Review performance subagent
1 parent 126aae6 commit 96c4022

File tree

3 files changed

+140
-1
lines changed

3 files changed

+140
-1
lines changed

‎.cursor/agents/review-performance.md‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
---
2+
name: review-performance
3+
description: Reviews code for performance patterns that affect UI responsiveness, especially RN JS thread load, YAOB bridge volume, and engine callback redundancy. Use when reviewing engine code, useWatch/withWallet, bridge/throttle, or navigation-related changes.
4+
---
5+
6+
Review the branch/pull request in context for performance conventions that affect wallet scene transitions and steady-state UI responsiveness.
7+
8+
## Context Expected
9+
10+
You will receive:
11+
- Repository name
12+
- Branch name
13+
- List of changed files to review
14+
15+
## How to Review
16+
17+
1. Read the changed files provided in context
18+
2. Look for engine callbacks, useWatch/subscription patterns, bridge/throttle usage, and navigation
19+
3. Verify changes avoid redundant callbacks and broad subscriptions that flood the RN JS thread
20+
4. Report findings with specific file:line references
21+
22+
---
23+
24+
## Avoid Redundant Engine Callbacks
25+
26+
Engine callbacks (onSyncStatusChanged, onBlockHeightChanged, onTransactions, onNewTokens, etc.) become YAOB bridge messages and Redux dispatches. Each one adds work on the RN JS thread. Do not fire when the value has not changed.
27+
28+
### onSyncStatusChanged — Gate by Ratio
29+
30+
Engines that are already fully synced (ratio=1.0) must not keep firing onSyncStatusChanged. Guard with last-value comparison:
31+
32+
```typescript
33+
// Incorrect - fires every poll even when ratio unchanged
34+
this.currencyEngineCallbacks.onSyncStatusChanged({ ratio })
35+
36+
// Correct - only fire when ratio actually changed
37+
if (ratio !== this.lastSyncRatio) {
38+
this.lastSyncRatio = ratio
39+
this.currencyEngineCallbacks.onSyncStatusChanged({ ratio })
40+
}
41+
```
42+
43+
### onBlockHeightChanged — Prefer onSeenTx / Direct Confirmations
44+
45+
Accountbased engines should set confirmations directly on transactions and use onSeenTxCheckpoint instead of calling onBlockHeightChanged every block. Every onBlockHeightChanged triggers core-js to iterate all transactions for that wallet. If confirmations are set on the transaction (e.g. 'confirmed' | 'unconfirmed' | number), core-js can skip that work.
46+
47+
- Set `confirmations: 'confirmed' | 'unconfirmed' | number` on EdgeTransaction in onTransactions
48+
- Call onSeenTxCheckpoint with block height when sync advances
49+
- Do not call onBlockHeightChanged for accountbased engines that set confirmations
50+
51+
UTXO plugins already set confirmations in toEdgeTransaction; they should not wire BLOCK_HEIGHT_CHANGED to callbacks.onBlockHeightChanged (internal event is fine for unconfirmed tx checks).
52+
53+
### onTransactions — Skip Emit for Unchanged / Already-Known
54+
55+
Do not emit TRANSACTIONS (or call onTransactions) when the transaction already exists and has not changed. In UTXO processAddressForTransactions, only emit if the tx is new or blockHeight changed:
56+
57+
```typescript
58+
// Correct - only emit when tx is new or changed
59+
if (existingTx == null || existingTx.blockHeight !== tx.blockHeight) {
60+
common.emitter.emit(EngineEvent.TRANSACTIONS, [{ isNew, transaction: edgeTx }])
61+
}
62+
```
63+
64+
In core-js currency-wallet-callbacks, gate the Redux CURRENCY_ENGINE_CHANGED_TXS dispatch so it does not fire when compare() filters out all transactions (no changed/created/txidHashes).
65+
66+
### onNewTokens — Gate by Actual Change
67+
68+
Only call onNewTokens when the detected token set has changed. Persist last-reported set (e.g. in walletLocalData as detectedTokenIds) and compare before calling:
69+
70+
```typescript
71+
// Correct - gate by sorted comparison, persist for cross-session
72+
protected reportDetectedTokens(tokenIds: string[]): void {
73+
const sorted = [...tokenIds].sort()
74+
if (sorted.length === this.lastDetectedTokenIds.length &&
75+
sorted.every((id, i) => id === this.lastDetectedTokenIds[i])) return
76+
this.lastDetectedTokenIds = sorted
77+
this.walletLocalData.detectedTokenIds = sorted
78+
this.currencyEngineCallbacks.onNewTokens(sorted)
79+
}
80+
```
81+
82+
Call sites (EthereumNetwork, AlgorandEngine, SolanaEngine, etc.) should use this helper instead of calling onNewTokens(detectedTokenIds) directly on every sync cycle.
83+
84+
### onStakingStatusChanged — Gate by Value Change
85+
86+
Do not call onStakingStatusChanged on every poll when status is unchanged. Compare JSON or key fields to last value and only fire when different (e.g. TronEngine, FioEngine).
87+
88+
### onSeenTxCheckpoint — Only on Advancement
89+
90+
Only call onSeenTxCheckpoint when the new checkpoint is higher than the previous one (e.g. MoneroEngine checkTransactionsInnerLoop). Do not fire on every loop when checkpoint is unchanged.
91+
92+
### Empty or No-Op Callbacks
93+
94+
Do not emit transaction callbacks with count=0 when there are no new or changed transactions (e.g. Monero empty onTransactions). Gate so only meaningful updates are sent.
95+
96+
---
97+
98+
## EngineEmitter and Event Deduplication
99+
100+
The EngineEmitter (and equivalent patterns) should suppress duplicate emissions when the value has not changed:
101+
102+
- BLOCK_HEIGHT_CHANGED: suppress if blockHeight === lastBlockHeight
103+
- WALLET_BALANCE_CHANGED: suppress if balance === lastBalance
104+
- ADDRESSES_CHECKED: suppress if ratio === lastRatio (and avoid resetting progress to 0 on every start() when wallet was already fully synced)
105+
106+
UTXO: If processedPercent was 1 (fully synced), do not reset to 0 on start(); skip the 0%→100% address-checked walk to avoid hundreds of redundant ADDRESSES_CHECKED events.
107+
108+
---
109+
110+
## Selective YAOB / useWatch Subscription
111+
112+
Components that only care about a single wallet must not subscribe to the entire account.currencyWallets map; that causes re-renders on any wallet change across all wallets.
113+
114+
- Prefer per-wallet useWatch(wallet, 'balance') or specific keys instead of useWatch(account, 'currencyWallets')
115+
- Use useMemo or selector patterns so the component does not re-render when the specific wallet data has not changed
116+
- In wallet list, consider subscribing only to wallets in the viewport
117+
118+
Locations: withWallet.tsx, WalletListSwipeable*, WalletDetailsScene, useWatch usage.
119+
120+
---
121+
122+
## Redux Dispatch Only When State Actually Changed
123+
124+
In core-js (e.g. currency-wallet-callbacks), avoid dispatching CURRENCY_ENGINE_CHANGED_* when the derived state has not changed. Each dispatch triggers the pixie watcher and yaob:update(walletApi). Gate dispatches on actual changes (e.g. changed/created/txidHashes length or content).
125+
126+
---
127+
128+
## Summary Checklist
129+
130+
- [ ] onSyncStatusChanged / onBlockHeightChanged / onNewTokens / onStakingStatusChanged / onSeenTxCheckpoint only fire when value changed
131+
- [ ] onTransactions / TRANSACTIONS not emitted for already-known unchanged txs
132+
- [ ] No empty or no-op transaction/sync callbacks (e.g. count=0 when nothing new)
133+
- [ ] EngineEmitter or equivalent deduplicates BLOCK_HEIGHT_CHANGED, WALLET_BALANCE_CHANGED, ADDRESSES_CHECKED
134+
- [ ] useWatch scoped to the minimal data needed (per-wallet, not whole currencyWallets)
135+
- [ ] Redux dispatch gated so it does not run when compare() or logic shows no user-visible change

‎.cursor/skills/review-code/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ For each entry in `manifest.subagents`:
4040
- **Skip** if the value is `false` or an empty array `[]`.
4141
- **Launch** if the value is `true` (no file filtering) or a non-empty array (those specific files).
4242

43-
Use the Task tool with the matching `subagent_type` (e.g. `review-react`, `review-async`, etc.). Launch up to 4 in parallel; batch the rest.
43+
Use the Task tool with the matching `subagent_type` (e.g. `review-react`, `review-async`, `review-performance`, etc.). Launch up to 4 in parallel; batch the rest.
4444

4545
Each subagent prompt must include:
4646

‎.cursor/skills/review-code/scripts/review-prep.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,10 @@ const subagents = {
305305

306306
'review-servers': isServer ? changedFiles : false,
307307

308+
'review-performance': filesWithPattern(
309+
/\b(useWatch|withWallet|YAOB|onBlockHeightChanged|onSyncStatusChanged|onNewTokens|onTransactions|EngineEmitter|BLOCK_HEIGHT_CHANGED|ADDRESSES_CHECKED|TRANSACTIONS|updateBlockHeight|onSeenTxCheckpoint|reportDetectedTokens|onStakingStatusChanged|InteractionManager|transitionStart|currencyWallets|saveWalletLoop|updateQueue|makeEngineEmitter)\b/
310+
),
311+
308312
'review-code-quality': changedFiles,
309313
'review-comments': changedFiles,
310314
'review-tests': changedFiles,

0 commit comments

Comments
 (0)