Repository navigation
feat(perps): support partial position reduction - #6725
SeniorZhai wants to merge 9 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e534988f15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| perpsPositionDao.deleteById(positionId) | ||
| } | ||
| Timber.d("Perps order closed: $positionId") | ||
| refreshSinglePosition(positionId) |
There was a problem hiding this comment.
Retain cache cleanup for full closes
When an existing full-close caller passes quantity == null, this replaces the previous cache deletion with refreshSinglePosition(). If the detail endpoint returns null or NOT_FOUND after the position has closed—both cases are explicitly ignored by refreshSinglePosition at lines 833–838—the cached row remains in an open state, and the revised order synchronization only invokes the same refresh. Consequently, observeOpenPositions continues showing the closed position until a separate wallet-wide refresh happens; keep the deletion behavior for full closes or remove/close the row when detail confirms it is gone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d4e9b1d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onToggleMode = { | ||
| input = (if (isPercentage) amount else percentage)?.let { formatMarginAdjustmentInput(it, !isPercentage) }.orEmpty() | ||
| isPercentage = !isPercentage |
There was a problem hiding this comment.
Preserve the reduction amount when switching input modes
Switching modes rewrites the input through formatMarginAdjustmentInput, which truncates percentages to an integer and dollar amounts to two decimals. For example, a $199.90 reduction on a $200 position becomes 99% after toggling to percentage mode, silently changing the submitted quantity to $198; small positions can similarly become $0 when toggled to amount mode. Keep the calculated quantity or an unrounded canonical amount across display-mode changes.
Useful? React with 👍 / 👎.
| if (isReducePosition) { | ||
| parentFragmentManager.setFragmentResult( | ||
| RESULT_POSITION_REDUCED, | ||
| Bundle().apply { putString(RESULT_POSITION_ID, positionId) }, | ||
| ) |
There was a problem hiding this comment.
Dismiss the reduction flow on every successful submission
The reduction result is emitted only from the Done button handler. Since the successful confirmation dialog remains cancelable with the system Back action, a user can dismiss it after Step.Done without notifying the parent reduction sheet; that sheet remains open and can submit another reduction against the remaining position. Emit the result when submission succeeds, or also propagate it when a completed dialog is dismissed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 198e4de513
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (input.length > 40 || input.any { it !in '0'..'9' && it != '.' }) return null | ||
| val quantity = input.toBigDecimalOrNull() ?: return null | ||
| val current = currentQuantity.toBigDecimalOrNull()?.abs() ?: return null | ||
| return quantity.takeIf { it > BigDecimal.ZERO && it <= current } |
There was a problem hiding this comment.
Reject stale reductions that become full closes
When a user selects a partial reduction and the position subsequently shrinks to exactly that requested quantity—for example, because another order or device updated it—the authoritative pre-submit check still passes because equality is allowed here. The request then closes the entire remaining position instead of reporting perps_reduce_quantity_changed; preserve the original position size or full-close intent and only permit equality when the user explicitly selected a 100% reduction.
Useful? React with 👍 / 👎.
| @SerializedName("quantity") | ||
| val quantity: String? = null, |
There was a problem hiding this comment.
Label partial close orders as reductions
When quantity is smaller than the open position, the resulting order is still ingested as PerpsOrder.TYPE_CLOSE, while ClosedActivityItem and PositionDetailPage unconditionally label every such order as “Closed Long” or “Closed Short.” Consequently, after a successful partial reduction the activity list and order detail claim that the position was closed even though it remains open; carry or derive partial-close metadata and render a reduction-specific label.
Useful? React with 👍 / 👎.
|
@copilot resolve the merge conflicts in this pull request |
…osition # Conflicts: # app/src/main/java/one/mixin/android/ui/home/web3/trade/perps/PositionDetailPage.kt # app/src/main/res/values-zh-rCN/strings.xml # app/src/main/res/values-zh-rTW/strings.xml Co-authored-by: SeniorZhai <5416585+SeniorZhai@users.noreply.github.com>
Merged |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 708bbe2bcb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| perpsPositionDao.deleteById(positionId) | ||
| } | ||
| Timber.d("Perps order closed: $positionId") | ||
| refreshSinglePosition(positionId) |
There was a problem hiding this comment.
Preserve the wallet ID when refreshing the position
When the position-detail endpoint returns an empty wallet_id, this new call uses the default null argument, so refreshSinglePosition persists remotePosition.walletId directly and overwrites the cached wallet ID with an empty string. The code already handles this endpoint behavior in loadPositionDetail by falling back to the local wallet ID, and open-position queries filter by the active wallet; consequently, a successfully reduced position can disappear or oscillate in the UI as position and order refreshes race. Pass the known/local wallet ID here or make refreshSinglePosition preserve it when the response value is blank.
Useful? React with 👍 / 👎.
| PerpsAdjustBottomSheetDialogFragment.ACTION_REDUCE_POSITION -> { | ||
| if (activity.supportFragmentManager.findFragmentByTag(PerpsReduceBottomSheetDialogFragment.TAG) == null) { | ||
| PerpsReduceBottomSheetDialogFragment.newInstance(position) | ||
| .show(activity.supportFragmentManager, PerpsReduceBottomSheetDialogFragment.TAG) |
There was a problem hiding this comment.
Track reduction starts before emitting close funnel events
When the user enters the new Reduce Position flow through this branch, no analytics start event is emitted, but the confirmation dialog records trade_perps_close_preview_confirm/cancel and a successful submission records trade_perps_close_end. Each reduction therefore creates orphan close-funnel events without a matching trade_perps_close_start, inflating close conversions and corrupting funnel results; either add a dedicated reduction funnel or emit a matching typed start event here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0da096a114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <string name="perps_reduce_quantity_changed">The position has changed. Please return and check the quantity before trying again.</string> | ||
| <string name="perps_reduce_invalid_quantity">Enter a positive amount within the current position size.</string> | ||
| <string name="perps_estimated_pnl">Estimated PnL</string> | ||
| <string name="perps_reduction_estimate_note">Estimates exclude fees. Final proceeds depend on execution and settlement. The liquidation price shown is the current price.</string> |
There was a problem hiding this comment.
Align the reduction fee disclosure with the calculation
When estimated_close_fee is nonzero, the reduction preview already subtracts the proportional fee from Estimated Receive via estimatedPerpsCloseReturn, and displays that fee separately, but this note tells users that the estimates exclude fees. This makes the financial preview internally contradictory and can lead users to interpret the shown proceeds as pre-fee; either describe the receive estimate as fee-inclusive or stop deducting the fee from it.
Useful? React with 👍 / 👎.
0da096a to
be9c162
Compare
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
No description provided.