Repository navigation
feat(perps): show net realized pnl and store profit share - #6728
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c307d4f53d
ℹ️ 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".
| val roe: String, | ||
| @SerializedName("net_realized_pnl") | ||
| @ColumnInfo(name = "net_realized_pnl", defaultValue = "''") | ||
| val netRealizedPnl: String = "", |
There was a problem hiding this comment.
When an orders response omits any of these new properties—or explicitly returns null for an older order—Gson bypasses Kotlin constructor defaults, so the corresponding non-null property is actually null. upsertSyncedOrders() then inserts it into a TEXT NOT NULL column and the order refresh fails instead of falling back to the legacy PnL. Make the wire fields nullable and normalize them before persistence, or provide a deserializer that supplies the empty-string defaults.
Useful? React with 👍 / 👎.
| suspend fun getLatestUpdatedAt(): String? | ||
|
|
||
| @Query("SELECT SUM(CAST(realized_pnl AS REAL)) FROM perps_orders WHERE order_type = 'close'") | ||
| @Query("SELECT SUM(CAST(COALESCE(NULLIF(net_realized_pnl, ''), realized_pnl) AS REAL)) FROM perps_orders WHERE order_type = 'close'") |
There was a problem hiding this comment.
When net_realized_pnl contains a non-empty malformed value such as "invalid" or whitespace, the display helper falls back to realized_pnl, but this query selects the malformed value and SQLite casts it to 0. Consequently, the total PnL can disagree with every displayed order; both aggregate queries should apply the same numeric-validation fallback as realizedPnlForDisplay().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ecff48f7e
ℹ️ 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".
| connection.execSQL("ALTER TABLE `perps_orders` ADD COLUMN `net_realized_pnl` TEXT NOT NULL DEFAULT ''") | ||
| connection.execSQL("ALTER TABLE `perps_orders` ADD COLUMN `net_roe` TEXT NOT NULL DEFAULT ''") | ||
| connection.execSQL("ALTER TABLE `perps_orders` ADD COLUMN `profit_share_amount` TEXT NOT NULL DEFAULT ''") |
There was a problem hiding this comment.
On a 9→10 upgrade, every cached order receives empty net fields, but refreshOrders() continues requesting from getLatestUpdatedAt() and therefore only upserts orders returned after the existing high-water mark. Completed historical orders normally never change again, so their new values are never fetched and the history and aggregate permanently fall back to the legacy gross PnL. The migration needs to trigger a one-time full backfill or invalidate the cached orders so all migrated close orders can acquire their net PnL and profit-share data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Missing JSON fields can break order synchronization, and malformed net values make aggregate totals inconsistent with row displays.
2 open findings
What changed in this PR
Adds net realized PnL, net ROE, and profit-share persistence for perpetual orders, then uses net values throughout closed-position displays.
Changes:
- Adds Room schema v10 and migration for new order fields.
- Uses net PnL/ROE with legacy-value fallback.
- Updates DAO tests, migration tests, and Chinese labels.
| File | Description |
|---|---|
Room3DatabaseCompatibilityTest.kt |
Tests v9-to-v10 migration. |
PerpsOrderDaoTest.kt |
Tests storage and fallback behavior. |
values-zh-rTW/strings.xml |
Updates Traditional Chinese PnL labels. |
values-zh-rCN/strings.xml |
Updates Simplified Chinese PnL labels. |
PositionDetailPage.kt |
Displays net closed-position metrics. |
PerpsPositionShareBottomFragment.kt |
Uses net metrics in shared cards. |
ClosedActivityItem.kt |
Displays net closed-order metrics. |
ClosedPositionItem.kt |
Displays net closed-position metrics. |
PerpsDatabase.kt |
Adds schema version 10 migration. |
PerpsOrderDao.kt |
Aggregates net realized PnL. |
PerpsOrderItem.kt |
Exposes new persisted fields. |
PerpsOrder.kt |
Models new API/database fields. |
PerpsExt.kt |
Adds display fallback helpers. |
otherChannel/.../10.json |
Exports other-channel v10 schema. |
googlePlay/.../10.json |
Exports Google Play v10 schema. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| @SerializedName("net_realized_pnl") | ||
| @ColumnInfo(name = "net_realized_pnl", defaultValue = "''") | ||
| val netRealizedPnl: String = "", | ||
| @SerializedName("net_roe") | ||
| @ColumnInfo(name = "net_roe", defaultValue = "''") |
| suspend fun getLatestUpdatedAt(): String? | ||
|
|
||
| @Query("SELECT SUM(CAST(realized_pnl AS REAL)) FROM perps_orders WHERE order_type = 'close'") | ||
| @Query("SELECT SUM(CAST(COALESCE(NULLIF(net_realized_pnl, ''), realized_pnl) AS REAL)) FROM perps_orders WHERE order_type = 'close'") |
No description provided.