Skip to content

feat(perps): support partial position reduction - #6725

Open
SeniorZhai wants to merge 9 commits into
masterfrom
feat/perps-reduce-position
Open

SeniorZhai wants to merge 9 commits into
masterfrom
feat/perps-reduce-position

Conversation

@SeniorZhai

Copy link
Copy Markdown
Member

No description provided.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T09:27:14.738441Z 6ef8fcb New commits
🔒 Security Review ✅ Completed 2026-10-09T06:07:39.003523Z be9c162 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +169 to +171
onToggleMode = {
input = (if (isPercentage) amount else percentage)?.let { formatMarginAdjustmentInput(it, !isPercentage) }.orEmpty()
isPercentage = !isPercentage

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +531 to +535
if (isReducePosition) {
parentFragmentManager.setFragmentResult(
RESULT_POSITION_REDUCED,
Bundle().apply { putString(RESULT_POSITION_ID, positionId) },
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +44 to +45
@SerializedName("quantity")
val quantity: String? = null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@SeniorZhai SeniorZhai added the testing Now testing, but you can review label Oct 9, 2026
@SeniorZhai

Copy link
Copy Markdown
Member Author

@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>

Copilot AI commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Merged master and resolved the conflicts in commit 708bbe2. The targeted Gradle test was blocked because Android Gradle Plugin 9.4.1 could not be resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +155 to +158
PerpsAdjustBottomSheetDialogFragment.ACTION_REDUCE_POSITION -> {
if (activity.supportFragmentManager.findFragmentByTag(PerpsReduceBottomSheetDialogFragment.TAG) == null) {
PerpsReduceBottomSheetDialogFragment.newInstance(position)
.show(activity.supportFragmentManager, PerpsReduceBottomSheetDialogFragment.TAG)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@SeniorZhai
SeniorZhai force-pushed the feat/perps-reduce-position branch from 0da096a to be9c162 Compare October 9, 2026 06:05
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Now testing, but you can review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants