fix(portfolio): include closed orders + render USD via base-currency hooks + LLM trade form polish#102
Merged
fray-cloud merged 2 commits intofeat/#98-position-reconcilerfrom May 2, 2026
Conversation
…a base-currency hooks Two bugs the user hit when checking testnet portfolio after #98 landed. 1. Realized PnL on the testnet card was always 0 because PortfolioService only queried status='filled'. The close-saga and reconciler both flip status to 'closed' on settlement, so every TP/SL/manual close fell out of the rollup. Fix: WHERE status IN ('filled','closed') so realizedPnl is summed from settled rows. 2. Backend `valueKrw` was misnamed — for futures it's USDT × quantity, i.e. USD. The frontend then ran formatKrw(usdAmount) and printed those USD figures with comma-separators as if they were KRW, so KRW display was effectively broken everywhere (and broke harder when the exchange-rate cache was empty). Backend - Order query in PortfolioService.getSummary now includes 'closed'. - Field rename valueKrw→valueUsd (also totalValueKrw→totalValueUsd) so the contract is honest about the unit. Same in DTOs and the frontend types. Frontend - New <MoneyValue usd={n}/> and <PnlMoney usd={n}/> shared components. They read useBaseCurrency + useExchangeRate themselves and route through formatCurrency, so a USD number displays correctly in KRW or USD with a sub-label of the alternate currency. When krwPerUsd=0 (rate not yet loaded) they fall back to USD-only — no more misnamed KRW. - Replaced every formatKrw(usd) / <PnlValue usd> usage: - /portfolio: total / realized / unrealized cards, byNetwork breakdown, asset-table, asset-card-list - /dashboard: today/week PnL cards, open positions table (entry, mark, unrealizedPnl) - /orders/[id]: entry / mark / TP / SL / realized / unrealized rows - Deleted the orphaned <PnlValue> + its test/stories. - demoPortfolio mock updated to USD scale. Tests - New: <MoneyValue> (KRW main + USD sub, USD main + KRW sub, no rate, null) and <PnlMoney> (sign, color, null) — 7 tests. - All existing api/worker/web suites still green (57 + 13 + 43). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three usability gaps reported on the LLM trade form: 1. TP/SL prices were just numbers — user couldn't tell at a glance how far they were from entry. Now each input shows raw price-distance % plus profit/ROE % (multiplied by leverage), with sign-corrected for short positions so a TP below entry on a short reads as +profit. 2. Leverage was a 1-20 number input that didn't match Binance's actual 1x-125x range or its preferred steps. Replaced with a 1-125 slider plus tick-buttons at 1/25/50/75/100/125 for one-click jumps. 3. Bet input was unbounded — user had no idea what their actual USDT balance was. Added a network toggle (testnet/mainnet), wires up the matching exchange key, fetches its balances via the existing /exchange-keys/:id/balances endpoint, displays free/total USDT, and adds 10/25/50/100% quick-fill buttons. Bet is clamped to free balance and the execute button is disabled when over. Backend: GetExchangeKeysHandler now selects `network` so the frontend can pick the right key per network without an extra round-trip. Tests still green (api 57, worker 13, web 43). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideBackend now computes and returns portfolio values strictly in USD (including closed orders for realized PnL), and the frontend renders all monetary values via new base-currency-aware MoneyValue/PnlMoney components while also upgrading the LLM trade form with network-aware key selection, balance-aware betting, and TP/SL leverage-aware ROE hints. Sequence diagram for LLM trade form with network-aware keys and balance-aware bettingsequenceDiagram
actor Trader
participant LlmTradeForm
participant UseExchangeKeys
participant ApiServer
participant GetExchangeKeysHandler
participant UseBalances
participant BalanceApi
participant ExecuteTradeApi
Trader->>LlmTradeForm: Open LlmTradeForm
activate LlmTradeForm
LlmTradeForm->>UseExchangeKeys: useExchangeKeys()
UseExchangeKeys->>ApiServer: GET /exchange-keys
ApiServer->>GetExchangeKeysHandler: GetExchangeKeysQuery
GetExchangeKeysHandler-->>ApiServer: ExchangeKeyItem[] (id, exchange, network,...)
ApiServer-->>UseExchangeKeys: ExchangeKeyItem[]
UseExchangeKeys-->>LlmTradeForm: keys
LlmTradeForm->>LlmTradeForm: selectKey = keys.find(exchange=binance, network=testnet)
LlmTradeForm->>UseBalances: useBalances(selectKey.id)
UseBalances->>BalanceApi: GET /balances?exchangeKeyId
BalanceApi-->>UseBalances: balances[USDT]
UseBalances-->>LlmTradeForm: balances
LlmTradeForm->>LlmTradeForm: usdtFree, usdtBalance via useMemo
LlmTradeForm->>LlmTradeForm: useEffect clamp betUsdt to usdtFree
Trader->>LlmTradeForm: Toggle network mainnet
LlmTradeForm->>LlmTradeForm: network = mainnet
LlmTradeForm->>LlmTradeForm: selectedKey = keys.find(network=mainnet)
LlmTradeForm->>UseBalances: useBalances(selectedKey.id)
UseBalances->>BalanceApi: GET /balances?exchangeKeyId
BalanceApi-->>UseBalances: balances[USDT]
UseBalances-->>LlmTradeForm: balances
LlmTradeForm->>LlmTradeForm: update usdtFree, usdtBalance
Trader->>LlmTradeForm: Adjust leverage slider, TP, SL, betUsdt
LlmTradeForm->>LlmTradeForm: tpPct, slPct, tpProfitPct, slLossPct, ROE
Trader->>LlmTradeForm: Click Execute
LlmTradeForm->>LlmTradeForm: Check selectedKey && betUsdt <= usdtFree
LlmTradeForm-->>Trader: Disable if invalid
LlmTradeForm->>ExecuteTradeApi: POST /execute-trade (exchangeKeyId, network, betUsdt, leverage, TP, SL)
ExecuteTradeApi-->>LlmTradeForm: Execution result
LlmTradeForm-->>Trader: Show success / error message
deactivate LlmTradeForm
Sequence diagram for portfolio summary including closed orders and USD valuessequenceDiagram
actor User
participant WebApp
participant ApiClient
participant ApiServer
participant PortfolioService
participant Prisma
User->>WebApp: Open portfolio / dashboard
WebApp->>ApiClient: getPortfolioSummary(network)
ApiClient->>ApiServer: GET /portfolio?network
ApiServer->>PortfolioService: getSummary(userId, network)
PortfolioService->>Prisma: order.findMany(where userId, status in [filled, closed])
Prisma-->>PortfolioService: orders with realizedPnl and network
PortfolioService->>PortfolioService: buildAvgCostMap(rows)
PortfolioService->>PortfolioService: dailyDeltaMap(rows)
PortfolioService->>Prisma: balance and price queries
Prisma-->>PortfolioService: balances, prices
PortfolioService->>PortfolioService: Compute valueUsd = currentPrice * quantity
PortfolioService->>PortfolioService: Compute pnl per asset
PortfolioService->>PortfolioService: Aggregate totalValueUsd, realizedPnl, unrealizedPnl by network
PortfolioService-->>ApiServer: PortfolioSummaryResponse (totalValueUsd, byNetwork.totalValueUsd,...)
ApiServer-->>ApiClient: PortfolioSummary
ApiClient-->>WebApp: data
WebApp->>WebApp: Render MoneyValue and PnlMoney components
WebApp-->>User: Portfolio and dashboard with base-currency-aware values
Updated class diagram for portfolio types and money display componentsclassDiagram
class PortfolioService {
+getSummary(userId: string, network: string) Promise~PortfolioSummaryResponse~
-buildAvgCostMap(rows: any[]) Map~string, number~
-dailyDeltaMap(rows: any[]) Map~string, number~
-calculateRealizedPnl(rows: any[], avgCostMap: Map~string, number~) number
-toCumulative(deltas: Map~string, number~) DailyPnlItem[]
}
class PortfolioAsset {
+exchange: string
+currency: string
+network: string
+quantity: string
+avgCost: number
+currentPrice: number
+valueUsd: number
+pnl: number
}
class NetworkBreakdownResponse {
+totalValueUsd: number
+realizedPnl: number
+unrealizedPnl: number
+dailyPnl: DailyPnlItem[]
}
class DailyPnlItem {
+date: string
+pnl: number
}
class PortfolioSummaryResponse {
+network: string
+totalValueUsd: number
+realizedPnl: number
+unrealizedPnl: number
+assets: PortfolioAssetResponse[]
+byNetwork: ByNetworkResponse
}
class PortfolioAssetResponse {
+exchange: string
+currency: string
+network: string
+quantity: string
+avgCost: number
+currentPrice: number
+valueUsd: number
+pnl: number
}
class ByNetworkResponse {
+testnet: NetworkBreakdownResponse
+mainnet: NetworkBreakdownResponse
}
class ExchangeKeyItem {
+id: string
+exchange: string
+network: string
+createdAt: string
+updatedAt: string
}
class PortfolioSummary {
+network: string
+totalValueUsd: number
+realizedPnl: number
+unrealizedPnl: number
+assets: PortfolioAsset[]
+byNetwork: FrontendByNetwork
}
class FrontendByNetwork {
+testnet: NetworkBreakdown
+mainnet: NetworkBreakdown
}
class NetworkBreakdown {
+totalValueUsd: number
+realizedPnl: number
+unrealizedPnl: number
+dailyPnl: DailyPnlItem[]
}
class MoneyValue {
+usd: number
+className: string
+showSub: boolean
+render(): JSX.Element
}
class PnlMoney {
+usd: number
+className: string
+showSub: boolean
+render(): JSX.Element
}
class UseBaseCurrencyHook {
+currency: string
}
class UseExchangeRateHook {
+krwPerUsd: number
}
PortfolioService --> PortfolioSummaryResponse
PortfolioSummaryResponse *-- PortfolioAssetResponse
PortfolioSummaryResponse *-- ByNetworkResponse
ByNetworkResponse *-- NetworkBreakdownResponse
NetworkBreakdownResponse *-- DailyPnlItem
PortfolioAssetResponse o-- PortfolioAsset
PortfolioSummary o-- PortfolioSummaryResponse
PortfolioSummary *-- FrontendByNetwork
FrontendByNetwork *-- NetworkBreakdown
NetworkBreakdown *-- DailyPnlItem
MoneyValue ..> UseBaseCurrencyHook
MoneyValue ..> UseExchangeRateHook
PnlMoney ..> UseBaseCurrencyHook
PnlMoney ..> UseExchangeRateHook
LlmTradeForm ..> ExchangeKeyItem
PortfolioPage ..> MoneyValue
PortfolioPage ..> PnlMoney
AssetTable ..> MoneyValue
AssetTable ..> PnlMoney
AssetCardList ..> MoneyValue
AssetCardList ..> PnlMoney
class LlmTradeForm
class PortfolioPage
class AssetTable
class AssetCardList
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="apps/web/src/components/llm-trade/llm-trade-form.tsx" line_range="63-66" />
<code_context>
+ }, [balances]);
+
+ // Clamp bet to free balance whenever balance changes (and bet was over).
+ useEffect(() => {
+ if (usdtFree > 0 && betUsdt > usdtFree) setBetUsdt(Math.floor(usdtFree));
+ }, [usdtFree, betUsdt]);
+
const signalMutation = useMutation({
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The bet size clamping effect can set `betUsdt` below the input `min` and does more work than necessary.
Right now the effect can set `betUsdt` to `0` when `0 < usdtFree < 1`, because it uses `Math.floor(usdtFree)` without respecting the `<input min={1}>`. That makes the React state and the HTML constraint disagree. Including `betUsdt` in the dependency array also causes unnecessary re-renders, since the clamp only needs to react to balance changes.
You can align the state with the declared `min` and simplify the effect to only depend on `usdtFree`:
```ts
useEffect(() => {
const maxBet = Math.max(1, Math.floor(usdtFree));
setBetUsdt((prev) => (prev > maxBet ? maxBet : prev));
}, [usdtFree]);
```
```suggestion
// Clamp bet to free balance whenever balance changes (and bet was over).
useEffect(() => {
const maxBet = Math.max(1, Math.floor(usdtFree));
setBetUsdt((prev) => (prev > maxBet ? maxBet : prev));
}, [usdtFree]);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+63
to
+66
| // Clamp bet to free balance whenever balance changes (and bet was over). | ||
| useEffect(() => { | ||
| if (usdtFree > 0 && betUsdt > usdtFree) setBetUsdt(Math.floor(usdtFree)); | ||
| }, [usdtFree, betUsdt]); |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The bet size clamping effect can set betUsdt below the input min and does more work than necessary.
Right now the effect can set betUsdt to 0 when 0 < usdtFree < 1, because it uses Math.floor(usdtFree) without respecting the <input min={1}>. That makes the React state and the HTML constraint disagree. Including betUsdt in the dependency array also causes unnecessary re-renders, since the clamp only needs to react to balance changes.
You can align the state with the declared min and simplify the effect to only depend on usdtFree:
useEffect(() => {
const maxBet = Math.max(1, Math.floor(usdtFree));
setBetUsdt((prev) => (prev > maxBet ? maxBet : prev));
}, [usdtFree]);
Suggested change
| // Clamp bet to free balance whenever balance changes (and bet was over). | |
| useEffect(() => { | |
| if (usdtFree > 0 && betUsdt > usdtFree) setBetUsdt(Math.floor(usdtFree)); | |
| }, [usdtFree, betUsdt]); | |
| // Clamp bet to free balance whenever balance changes (and bet was over). | |
| useEffect(() => { | |
| const maxBet = Math.max(1, Math.floor(usdtFree)); | |
| setBetUsdt((prev) => (prev > maxBet ? maxBet : prev)); | |
| }, [usdtFree]); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
실거래 운용 중 발견한 두 버그 + LLM trade 폼 UX 3가지 개선.
Bug 1 — 모의 실현 손익 항상 0
PortfolioService가status='filled'만 조회 → close-saga/reconciler가'closed'로 바꾼 행이 통계에서 빠짐.status IN ('filled','closed')로 변경Bug 2 — KRW 전반적으로 깨짐
백엔드
valueKrw는 사실USDT × qty= USD. 프론트가formatKrw(usd값)으로 콤마만 찍어서 USD를 KRW처럼 출력.valueKrw → valueUsd,totalValueKrw → totalValueUsd정직화<MoneyValue usd={n}/>+<PnlMoney usd={n}/>— 내부에서useBaseCurrency+useExchangeRate통해 KRW/USD 토글에 반응. 환율 미로딩 시 USD 단독 fallback./portfolio,/dashboard,/orders/[id]의 모든 USD 값 (총가치/실현/미실현/현재가/평균가/마크/PnL)LLM trade form 개선
Backend
GetExchangeKeysHandler가network까지 select (프론트가 키 선택)Tests
<MoneyValue>4개 +<PnlMoney>3개🤖 Generated with Claude Code
Summary by Sourcery
Fix portfolio PnL calculation to include closed orders and standardize portfolio values as USD/USDT while introducing base-currency-aware money components and polishing the LLM trade form UX.
Bug Fixes:
Enhancements:
Tests: