Skip to content

feat: UI/UX overhaul — testnet/mainnet split, USD/KRW toggle, /orders/[id], dashboard rebuild#100

Merged
fray-cloud merged 1 commit intofeat/llm-trade-claude-clifrom
feat/#97-ui-overhaul
May 2, 2026
Merged

feat: UI/UX overhaul — testnet/mainnet split, USD/KRW toggle, /orders/[id], dashboard rebuild#100
fray-cloud merged 1 commit intofeat/llm-trade-claude-clifrom
feat/#97-ui-overhaul

Conversation

@fray-cloud
Copy link
Copy Markdown
Owner

@fray-cloud fray-cloud commented May 2, 2026

Closes #97.

PR3로 LLM 트레이드 흐름을 사용하기 시작했지만 주변 UI가 미완성이라 일상적인 운영이 어려운 4가지 문제를 한 번에 해결.

Backend

  • Portfolio: paper/real/all → testnet/mainnet/all (ExchangeKey.network 기반); byNetwork 분할 합계 포함
  • Orders: GET /orders/:id 가 결정/마크가/미실현 PnL까지 hydrate; POST /orders/:id/close reduceOnly MARKET 종료
  • Worker close saga: 거래소 포지션이 이미 사라졌으면(-2022 등) DB만 reconcile
  • LLM trades: GET /llm-trades/decisions 커서 페이지네이션 + 결과 outcome 조인
  • Dashboard: /dashboard/summary 단일 집계 엔드포인트
  • Activity: 주문 항목 링크 /orders/orders/:id

Frontend

  • nav-bar <BaseCurrencyToggle> (KRW⇄USD, localStorage)
  • formatCurrency helper {main, sub} 반환
  • /portfolio 네트워크 토글, 모의/실거래 분할, 자산 네트워크 배지
  • /orders/[id] 진입/TP/SL 가격라인 캔들 차트, PnL 패널, 수동 종료, LLM 결정 카드
  • /dashboard 오늘/이번주 PnL × 네트워크, 활성 포지션 테이블 인라인 종료, 최근 5개 LLM 결정 outcome 배지

Tests

  • 16 api-server + 1 worker + 7 web 슈트 (61 + 3 + 41) 그린

🤖 Generated with Claude Code

Summary by Sourcery

Introduce network-aware portfolio, order close flow, and a rebuilt dashboard with richer LLM trading insights.

New Features:

  • Expose portfolio summaries split by network (testnet/mainnet/all) including per-network breakdowns.
  • Add dashboard summary API aggregating today/week PnL by network, open positions with mark prices, and recent LLM decisions.
  • Introduce order detail view with charted entry/TP/SL levels, live PnL, manual close action, and attached LLM decision context.
  • Add client-side USD/KRW base currency toggle and currency formatting utilities for displaying values in dual currencies.
  • Expose cursor-paginated LLM decision listing and order detail API with mark price and unrealized PnL.

Bug Fixes:

  • Treat futures long/short sides correctly in cost basis, realized PnL, and daily PnL calculations, preferring exchange-reported realized PnL when available.
  • Ensure manual close requests gracefully reconcile already-closed exchange positions instead of failing.

Enhancements:

  • Refactor portfolio aggregation to be network-aware and return cumulative daily PnL plus per-network breakdown metadata.
  • Wire activity feed order items to deep-link into the new per-order detail page instead of the generic orders list.
  • Rebuild the dashboard UI to show network-split PnL, inline-closing of active positions, and recent LLM decision outcomes.
  • Extend worker and Kafka contracts to support a dedicated order-close saga handling reduce-only market exits and related notifications.
  • Add tests covering portfolio network filtering, order close command behavior, activity links, currency formatting, and enriched order queries.

Tests:

  • Add unit tests for portfolio service network filtering and PnL aggregation, order close command validation and Kafka emission, activity link generation, order enrichment handler, and currency formatting.

…/[id], dashboard rebuild (#97)

Resolves the four day-to-day operational gaps identified in #97 so that the
post-PR3 LLM trading flow is actually usable.

Backend
- Portfolio: replace stale paper/real/all mode with testnet/mainnet/all driven
  by ExchangeKey.network; returns byNetwork breakdown so the UI can show split
  totals in one call.
- Orders: GET /orders/:id now hydrates the row with the joined LLM decision,
  live mark price, and unrealized PnL; new POST /orders/:id/close publishes a
  Kafka close event consumed by a worker saga that calls reduceOnly MARKET
  closePosition.
- Worker close saga also reconciles when the position is already gone on the
  exchange (TP/SL fired or liquidation): pre-checks getPosition and treats
  -2022/-4046/-2023/-4045 as "position gone" so the DB still flips to closed.
- LLM trades: GET /llm-trades/decisions cursor-paginated history with order
  outcome joined.
- Dashboard: new /dashboard/summary aggregate (today/week PnL split by network,
  open positions with live mark, last 5 LLM decisions) — single round-trip.
- Activity: order item link now points to /orders/${id} (was the deleted
  /orders index route).

Frontend
- BaseCurrencyToggle pill in the global nav bar (KRW⇄USD, localStorage).
- New formatCurrency helper returning {main, sub} so every price renderer can
  show primary + alt currency without bespoke math.
- /portfolio: testnet/mainnet/all toggle, 모의/실거래 split totals, network
  badges on assets.
- /orders/[id]: candle chart with entry/TP/SL price lines, PnL panel, manual
  Close Position button (gated to real-mode + open), LLM reasoning card.
- /dashboard: PnL cards (today/week × testnet/mainnet), active positions
  table with one-click close, last 5 LLM decisions with outcome badges.

Tests
- 16 api-server + 1 worker + 7 web suites green.
- New: portfolio network filter, close-order kafka emit, activity link,
  order detail payload, close-position-saga reconcile paths (TP/SL gone,
  -2022 rejection, idempotency).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
coin-web Ready Ready Preview, Comment May 2, 2026 6:07am

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 2, 2026

Reviewer's Guide

Splits portfolio and PnL by testnet/mainnet instead of paper/real, adds richer order/dashboards endpoints (including /orders/:id and /dashboard/summary), wires a Kafka-based manual close-position saga, and overhauls the web UI with network-aware portfolio views, an order detail page with chart/PnL, and a global KRW/USD toggle.

Sequence diagram for manual close-position flow via Kafka saga

sequenceDiagram
  actor User
  participant WebApp
  participant ApiServer_OrdersController
  participant CloseOrderHandler
  participant Kafka
  participant WorkerService_OrdersService
  participant ClosePositionSaga
  participant Exchange
  participant PrismaDB
  participant NotificationTopic

  User->>WebApp: Click close position button
  WebApp->>ApiServer_OrdersController: POST /orders/{id}/close
  ApiServer_OrdersController->>CloseOrderHandler: execute(userId, orderId)
  CloseOrderHandler->>PrismaDB: order.findFirst(userId, orderId)
  PrismaDB-->>CloseOrderHandler: order
  CloseOrderHandler->>Kafka: send(TRADING_ORDER_CLOSE_REQUESTED, OrderCloseRequestedEvent)
  CloseOrderHandler-->>ApiServer_OrdersController: {id, status: pending}
  ApiServer_OrdersController-->>WebApp: 202 Accepted
  WebApp-->>User: Show close requested message

  Kafka-->>WorkerService_OrdersService: TRADING_ORDER_CLOSE_REQUESTED event
  WorkerService_OrdersService->>ClosePositionSaga: executeClosePositionSaga(event, prisma, producer, redis)
  ClosePositionSaga->>PrismaDB: order.findFirst(userId, dbOrderId)
  PrismaDB-->>ClosePositionSaga: order
  ClosePositionSaga->>Exchange: getPosition(credentials, symbol)
  Exchange-->>ClosePositionSaga: livePosition
  alt position exists
    ClosePositionSaga->>Exchange: closePosition(credentials, symbol, side, quantity)
    Exchange-->>ClosePositionSaga: closeResult
  else position already gone
    ClosePositionSaga->>PrismaDB: order.update(status closed, realizedPnl null)
  end
  ClosePositionSaga->>PrismaDB: order.update(status closed, realizedPnl)
  ClosePositionSaga->>Kafka: send(TRADING_ORDER_RESULT, OrderResultEvent)
  ClosePositionSaga->>Kafka: send(NOTIFICATION_SEND, NotificationEvent)
  Kafka-->>NotificationTopic: order_filled notification

  WebApp->>ApiServer_OrdersController: GET /orders/{id}
  ApiServer_OrdersController->>PrismaDB: order.findFirst with llmDecision, exchangeKey
  PrismaDB-->>ApiServer_OrdersController: order with decision and network
  ApiServer_OrdersController-->>WebApp: order detail with closedAt and realizedPnl
  WebApp-->>User: Update order detail and PnL
Loading

Class diagram for updated portfolio and dashboard models

classDiagram
  class PortfolioService {
    +getSummary(userId string, network PortfolioNetwork) Promise~PortfolioSummaryModel~
    -buildAvgCostMap(orders OrderArray) Map~string, number~
    -calculateRealizedPnl(orders OrderArray, avgCostMap Map~string, number~) number
    -dailyDeltaMap(orders OrderArray) Map~string, number~
    -toCumulative(deltas Map~string, number~) DailyPnlArray
  }

  class PortfolioNetwork {
    <<enumeration>>
    testnet
    mainnet
    all
  }

  class PortfolioAssetModel {
    +exchange string
    +currency string
    +network string
    +quantity string
    +avgCost number
    +currentPrice number
    +valueKrw number
    +pnl number
  }

  class NetworkBreakdownModel {
    +totalValueKrw number
    +realizedPnl number
    +unrealizedPnl number
    +dailyPnl DailyPnlArray
  }

  class DailyPnlItemModel {
    +date string
    +pnl number
  }

  class PortfolioSummaryModel {
    +network PortfolioNetwork
    +totalValueKrw number
    +realizedPnl number
    +unrealizedPnl number
    +assets PortfolioAssetModelArray
    +dailyPnl DailyPnlArray
    +byNetwork ByNetworkModel
  }

  class ByNetworkModel {
    +testnet NetworkBreakdownModel
    +mainnet NetworkBreakdownModel
  }

  class PortfolioAssetResponse {
    +exchange string
    +currency string
    +network string
    +quantity string
    +avgCost number
    +currentPrice number
    +valueKrw number
    +pnl number
  }

  class NetworkBreakdownResponse {
    +totalValueKrw number
    +realizedPnl number
    +unrealizedPnl number
    +dailyPnl DailyPnlItemResponseArray
  }

  class DailyPnlItemResponse {
    +date string
    +pnl number
  }

  class PortfolioByNetworkResponse {
    +testnet NetworkBreakdownResponse
    +mainnet NetworkBreakdownResponse
  }

  class PortfolioSummaryResponse {
    +network string
    +totalValueKrw number
    +realizedPnl number
    +unrealizedPnl number
    +assets PortfolioAssetResponseArray
    +dailyPnl DailyPnlItemResponseArray
    +byNetwork PortfolioByNetworkResponse
  }

  class DashboardService {
    +getSummary(userId string) Promise~DashboardSummaryModel~
    -bucketByNetwork(rows OrderRowArray) NetworkPnlBucket
    -fetchMarkPrice(exchange string, symbol string) Promise~number~
    -computeUnrealizedPnl(order DashboardOrderModel, markPrice number) number
  }

  class DashboardSummaryModel {
    +pnl DashboardPnlBuckets
    +openPositions DashboardOpenPositionArray
    +recentDecisions DashboardDecisionArray
  }

  class DashboardPnlBuckets {
    +today NetworkPnlBucket
    +week NetworkPnlBucket
  }

  class NetworkPnlBucket {
    +testnet number
    +mainnet number
  }

  PortfolioService --> PortfolioSummaryModel
  PortfolioSummaryModel --> ByNetworkModel
  ByNetworkModel --> NetworkBreakdownModel
  PortfolioSummaryModel --> PortfolioAssetModel
  NetworkBreakdownModel --> DailyPnlItemModel

  PortfolioSummaryResponse --> PortfolioByNetworkResponse
  PortfolioByNetworkResponse --> NetworkBreakdownResponse
  NetworkBreakdownResponse --> DailyPnlItemResponse
  PortfolioSummaryResponse --> PortfolioAssetResponse

  DashboardService --> DashboardSummaryModel
  DashboardSummaryModel --> DashboardPnlBuckets
  DashboardPnlBuckets --> NetworkPnlBucket
Loading

Class diagram for order detail, getOrder handler, and close-position saga

classDiagram
  class OrderDetailModel {
    +order OrderEntity
    +decision LlmDecisionModel
    +network string
    +markPrice number
    +unrealizedPnl number
  }

  class OrderEntity {
    +id string
    +userId string
    +exchange string
    +symbol string
    +side string
    +status string
    +mode string
    +entryPrice string
    +takeProfitPrice string
    +stopLossPrice string
    +realizedPnl string
    +closedAt Date
    +leverage number
    +positionSide string
    +filledQuantity string
    +quantity string
  }

  class LlmDecisionModel {
    +id string
    +parsedSignal ParsedSignalModel
    +model string
    +latencyMs number
    +createdAt Date
  }

  class ParsedSignalModel {
    +signal string
    +takeProfitPrice string
    +stopLossPrice string
    +reasoning string
  }

  class GetOrderHandler {
    +execute(query GetOrderQuery) Promise~OrderDetailModel~
    -fetchMarkPrice(exchange string, symbol string) Promise~number~
    -computeUnrealizedPnl(order GetOrderPnlOrder, markPrice number) number
  }

  class CloseOrderCommand {
    +userId string
    +orderId string
  }

  class CloseOrderHandler {
    +execute(command CloseOrderCommand) Promise~CloseOrderResult~
    +onModuleInit() void
    +onModuleDestroy() void
    -kafka Kafka
    -producer Producer
  }

  class CloseOrderResult {
    +id string
    +status string
  }

  class OrderCloseRequestedEventModel {
    +requestId string
    +userId string
    +dbOrderId string
  }

  class OrderResultEventModel {
    +requestId string
    +userId string
    +dbOrderId string
    +mode string
  }

  class NotificationEventModel {
    +userId string
    +type string
    +title string
    +message string
  }

  class ClosePositionSagaFunction {
    +executeClosePositionSaga(event OrderCloseRequestedEventModel, prisma PrismaService, producer Producer, redis Redis) Promise~void~
    -isPositionGoneError(err Error) boolean
    -markOrderClosed(prisma PrismaService, orderId string, realizedPnl string) Promise~void~
    -emitClosedEvents(producer Producer, event OrderCloseRequestedEventModel, order CloseOrderSagaOrder, side PositionSide, quantity string, closeResult CloseResultModel, message string) Promise~void~
  }

  class DashboardOpenPositionModel {
    +entryPrice string
    +takeProfitPrice string
    +stopLossPrice string
    +leverage number
    +markPrice number
    +unrealizedPnl number
    +exchangeKey NetworkHolder
  }

  class NetworkHolder {
    +network string
  }

  OrderDetailModel --> OrderEntity
  OrderDetailModel --> LlmDecisionModel
  LlmDecisionModel --> ParsedSignalModel

  GetOrderHandler --> OrderDetailModel
  GetOrderHandler --> OrderEntity

  CloseOrderHandler --> CloseOrderCommand
  CloseOrderHandler --> OrderCloseRequestedEventModel
  ClosePositionSagaFunction --> OrderResultEventModel
  ClosePositionSagaFunction --> NotificationEventModel

  DashboardOpenPositionModel --> NetworkHolder
  OrderEntity <|-- DashboardOpenPositionModel
Loading

File-Level Changes

Change Details Files
Portfolio backend now aggregates balances and PnL by exchange key network (testnet/mainnet/all) and exposes per-network breakdowns to the API and web client.
  • Replaced mode-based summary logic with network-based filtering using exchangeKey.network and removed paper-mode virtual balance computation.
  • Builds avg cost and PnL separately for testnet and mainnet orders, then merges them (including cumulative daily PnL) into a combined response when network=all.
  • Extended PortfolioSummary/PortfolioAsset DTOs and web types to include network, byNetwork breakdowns, and updated controller/query wiring and tests accordingly.
apps/api-server/src/portfolio/portfolio.service.ts
apps/api-server/src/portfolio/dto/portfolio-response.dto.ts
apps/api-server/src/portfolio/queries/get-portfolio-summary.query.ts
apps/api-server/src/portfolio/queries/get-portfolio-summary.handler.ts
apps/api-server/src/portfolio/portfolio.controller.ts
apps/api-server/src/portfolio/portfolio.service.test.ts
apps/api-server/src/portfolio/queries/get-portfolio-summary.handler.test.ts
apps/web/src/lib/api-client.ts
apps/web/src/hooks/use-portfolio.ts
apps/web/src/mocks/data/portfolio.ts
Introduced an order-detail read model and a new /orders/:id UI that shows chart, prices, PnL, and LLM decision, plus activity links pointing to it.
  • GetOrderHandler now joins llmDecision and exchangeKey, pulls mark price from Redis, computes unrealized PnL (long/short aware), and wraps everything in a composite response structure.
  • Added tests for get-order handler covering mark price, long/short unrealized PnL behavior, closed orders, and not-found cases.
  • Created a new Next.js /orders/[id] page that fetches the enriched order payload, renders an OrderChart with entry/TP/SL lines, shows PnL/value panels, and surfaces LLM signal details.
  • Updated ActivityService and its tests so order activity items link to /orders/{id} instead of /orders.
apps/api-server/src/orders/queries/get-order.handler.ts
apps/api-server/src/orders/queries/get-order.handler.test.ts
apps/api-server/src/activity/activity.service.ts
apps/api-server/src/activity/activity.service.test.ts
apps/web/src/app/orders/[id]/page.tsx
apps/web/src/components/order-chart.tsx
Added a manual close-position flow for real orders via API, Kafka, and a worker saga, with reconciliation when the exchange position is already gone.
  • Defined OrderCloseRequestedEvent and TRADING_ORDER_CLOSE_REQUESTED topic in kafka-contracts.
  • Implemented CloseOrderCommand/Handler to validate eligibility (real-mode, filled, not already closed, has exchangeKey) and emit a Kafka close-request event; added tests for error paths and success.
  • Extended OrdersController with POST /orders/:id/close endpoint that dispatches the CloseOrderCommand.
  • Worker OrdersService now subscribes to the close-request topic, delegating to a new close-position saga.
  • Implemented executeClosePositionSaga in worker: acquires a Redis-based lock, loads DB order/exchangeKey, calls adapter.getPosition/closePosition, handles Binance "position gone" error codes by reconciling DB only, computes realized PnL heuristically, updates the order as closed, and emits TRADING_ORDER_RESULT and NOTIFICATION events.
  • Added (placeholder) test scaffold for the close-position saga.
packages/kafka-contracts/src/events.ts
packages/kafka-contracts/src/topics.ts
apps/api-server/src/orders/orders.controller.ts
apps/api-server/src/orders/commands/close-order.command.ts
apps/api-server/src/orders/commands/close-order.handler.ts
apps/api-server/src/orders/commands/close-order.handler.test.ts
apps/api-server/src/orders/commands/index.ts
apps/worker-service/src/orders/orders.service.ts
apps/worker-service/src/orders/sagas/close-position-saga.ts
apps/worker-service/src/orders/sagas/close-position-saga.test.ts
Introduced a consolidated dashboard summary backend endpoint and rebuilt the dashboard UI to show PnL by network, open positions with inline close, and recent LLM decisions.
  • Added DashboardModule with DashboardService and DashboardController; service queries today/week realized PnL, open real positions, and recent llmDecisionLog entries, buckets realized PnL by network, and decorates open positions with mark price and unrealized PnL using Redis tickers.
  • Exposed GET /dashboard/summary and wired it into the main AppModule.
  • On the web, defined DashboardSummary and LlmDecisionItem types plus getDashboardSummary API client.
  • Replaced the placeholder dashboard page with a React Query-driven page rendering PnlSection (today/week testnet/mainnet PnL), an OpenPositionsSection table (including network badges and inline close button wired to closePosition) and a RecentDecisionsSection with outcome badges and links to order-detail.
  • Added a small helper to derive user-visible LLM decision outcome labels from order status/realizedPnl.
apps/api-server/src/dashboard/dashboard.service.ts
apps/api-server/src/dashboard/dashboard.controller.ts
apps/api-server/src/dashboard/dashboard.module.ts
apps/api-server/src/app.module.ts
apps/web/src/lib/api-client.ts
apps/web/src/app/dashboard/page.tsx
Overhauled the portfolio UI to use testnet/mainnet/all toggles, show per-network summaries, and attach network metadata to assets.
  • Portfolio page now keeps a PortfolioNetwork state, queries getPortfolioSummary(network), and renders a three-way toggle with Korean labels and testnet/mainnet-colored active states.
  • When viewing the combined network, an additional split summary card row shows testnet/mainnet total value and realized/unrealized PnL using byNetwork from the API.
  • Assets now include a network field on the client side, and demoPortfolio/mocks updated accordingly.
apps/web/src/app/portfolio/page.tsx
apps/web/src/lib/api-client.ts
apps/web/src/mocks/data/portfolio.ts
apps/web/src/hooks/use-portfolio.ts
Added a global base-currency toggle (KRW/USD) and a shared formatter that renders values in the selected base with a secondary converted amount.
  • Introduced BaseCurrency type and formatCurrency helper, returning {main, sub} strings for KRW/USD, backed by exchange rate; added unit tests for output behavior.
  • Created component that reads/writes the current currency via useBaseCurrency and renders a small two-state pill control.
  • Wired BaseCurrencyToggle into the nav-bar and used formatCurrency in the dashboard open-positions table and order detail page when displaying entry/mark/TP/SL.
  • Ensured exchange-rate dependency via useExchangeRate hook integration where values are rendered.
apps/web/src/lib/utils.ts
apps/web/src/lib/utils.test.ts
apps/web/src/components/base-currency-toggle.tsx
apps/web/src/components/nav-bar.tsx
apps/web/src/app/dashboard/page.tsx
apps/web/src/app/orders/[id]/page.tsx
Extended LLM trades list API with cursor-based pagination and order joins, and exposed it via the existing controller.
  • LlmTradesService gained listDecisions(userId, limit, cursor) which fetches llmDecisionLog rows with optional createdAt cursor and an order projection, returning items plus nextCursor.
  • LlmTradesController now exposes GET /llm-trades/decisions with limit/cursor query params, parsing limit to a clamped 1–100 page size and delegating to listDecisions.
apps/api-server/src/llm-trades/llm-trades.service.ts
apps/api-server/src/llm-trades/llm-trades.controller.ts

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In PortfolioService.getSummary, buildAvgCostMap is called inside the loop over filteredKeys for each key; consider precomputing separate avg-cost maps for testnet/mainnet once (like you already do for PnL breakdown) and reusing them per key to avoid O(keys × orders) recomputation.
  • GetOrderHandler and DashboardService each create their own Redis instances and duplicate mark-price/unrealized-PnL logic; it would be more efficient and maintainable to extract a shared, injectable Redis/ticker service that centralizes connection management and PnL computation.
  • In close-position-saga.ts, the notification for a successful manual close uses type: 'order_filled'; if this is meant to be distinguished from initial fills, consider introducing a dedicated notification type (e.g. position_closed) to avoid overloading the existing semantics.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `PortfolioService.getSummary`, `buildAvgCostMap` is called inside the loop over `filteredKeys` for each key; consider precomputing separate avg-cost maps for testnet/mainnet once (like you already do for PnL breakdown) and reusing them per key to avoid O(keys × orders) recomputation.
- `GetOrderHandler` and `DashboardService` each create their own `Redis` instances and duplicate mark-price/unrealized-PnL logic; it would be more efficient and maintainable to extract a shared, injectable Redis/ticker service that centralizes connection management and PnL computation.
- In `close-position-saga.ts`, the notification for a successful manual close uses `type: 'order_filled'`; if this is meant to be distinguished from initial fills, consider introducing a dedicated notification type (e.g. `position_closed`) to avoid overloading the existing semantics.

## Individual Comments

### Comment 1
<location path="apps/worker-service/src/orders/sagas/close-position-saga.ts" line_range="82-86" />
<code_context>
+
+  // 1. If the position is already gone on Binance (TP/SL fired, liquidation,
+  //    or a previous close request succeeded), just reconcile our DB and stop.
+  const livePosBefore = await adapter.getPosition(credentials, order.symbol).catch((e) => {
+    logger.warn(`getPosition pre-check failed: ${e}`);
+    return null;
+  });
+  if (!livePosBefore) {
+    logger.warn(`Position already gone on exchange — reconciling order ${order.id}`);
+    await markOrderClosed(prisma, order.id, null);
</code_context>
<issue_to_address>
**issue (bug_risk):** Treating `getPosition` failures as "position gone" can incorrectly close live positions on transient errors.

Because all `getPosition` errors are coerced to `null`, a transient network/API failure looks identical to “no position,” and the order is closed even though the position may still be open on the exchange. This can silently desync our state. Consider handling “position not found” explicitly (e.g., by checking specific error codes) and treating other errors as failures that should bubble up or be retried, rather than collapsing them into the “position gone” path.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +82 to +86
const livePosBefore = await adapter.getPosition(credentials, order.symbol).catch((e) => {
logger.warn(`getPosition pre-check failed: ${e}`);
return null;
});
if (!livePosBefore) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Treating getPosition failures as "position gone" can incorrectly close live positions on transient errors.

Because all getPosition errors are coerced to null, a transient network/API failure looks identical to “no position,” and the order is closed even though the position may still be open on the exchange. This can silently desync our state. Consider handling “position not found” explicitly (e.g., by checking specific error codes) and treating other errors as failures that should bubble up or be retried, rather than collapsing them into the “position gone” path.

@fray-cloud fray-cloud merged commit 2268119 into feat/llm-trade-claude-cli May 2, 2026
3 checks passed
fray-cloud added a commit that referenced this pull request May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant