From bd74f9447fd42ceeae1e367c9c4b04ac629eb6cf Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:26:19 +0530 Subject: [PATCH 01/19] test(paper-trade): failing integration test + ListAllTrades DB helper --- cmd/api/paper_trade_pipeline_test.go | 59 ++++++++++++++++++++++++++++ internal/db/trades.go | 36 +++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 cmd/api/paper_trade_pipeline_test.go create mode 100644 internal/db/trades.go diff --git a/cmd/api/paper_trade_pipeline_test.go b/cmd/api/paper_trade_pipeline_test.go new file mode 100644 index 0000000..6d71a0b --- /dev/null +++ b/cmd/api/paper_trade_pipeline_test.go @@ -0,0 +1,59 @@ +//go:build integration + +package main + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPaperTrade_PersistsAllRows(t *testing.T) { + srv, _ := setupTestAPIServer(t) + + body := `{"symbol":"BTCUSDT","side":"BUY","type":"market","quantity":0.1,"price":45000}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/trade", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + srv.router.ServeHTTP(w, req) + + require.Equal(t, http.StatusCreated, w.Code) + var resp map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + orderID := resp["order"].(map[string]interface{})["id"].(string) + + ctx := context.Background() + + // Verify order row is filled with non-zero executed_quote_quantity + order, err := srv.db.GetOrder(ctx, uuid.MustParse(orderID)) + require.NoError(t, err) + assert.Equal(t, "FILLED", string(order.Status)) + assert.Greater(t, order.ExecutedQuantity, 0.0) + assert.Greater(t, order.ExecutedQuoteQuantity, 0.0, "executed_quote_quantity must be non-zero (price bug)") + + // Verify trade fill row via GetTradesByOrderID (already exists in orders.go) + fills, err := srv.db.GetTradesByOrderID(ctx, uuid.MustParse(orderID)) + require.NoError(t, err) + assert.NotEmpty(t, fills, "expected at least one trade fill row in trades table") + assert.Greater(t, fills[0].Price, 0.0, "fill price must be > 0") + + // Verify open position exists + positions, err := srv.db.GetAllOpenPositions(ctx) + require.NoError(t, err) + found := false + for _, p := range positions { + if p.Symbol == "BTCUSDT" { + found = true + assert.Greater(t, p.EntryPrice, 0.0) + assert.Greater(t, p.Quantity, 0.0) + } + } + assert.True(t, found, "expected BTCUSDT open position after paper trade") +} diff --git a/internal/db/trades.go b/internal/db/trades.go new file mode 100644 index 0000000..53d3d17 --- /dev/null +++ b/internal/db/trades.go @@ -0,0 +1,36 @@ +package db + +import ( + "context" +) + +// ListAllTrades returns recent trade fills across all orders, newest first. +// For fills by specific order, use GetTradesByOrderID in orders.go instead. +func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, error) { + rows, err := db.pool.Query(ctx, ` + SELECT id, order_id, exchange_trade_id, symbol, exchange, side, + price, quantity, quote_quantity, commission, commission_asset, + executed_at, is_maker, metadata, created_at + FROM trades + ORDER BY executed_at DESC + LIMIT $1 OFFSET $2 + `, limit, offset) + if err != nil { + return nil, err + } + defer rows.Close() + + var trades []*Trade + for rows.Next() { + t := &Trade{} + if err := rows.Scan( + &t.ID, &t.OrderID, &t.ExchangeTradeID, &t.Symbol, &t.Exchange, &t.Side, + &t.Price, &t.Quantity, &t.QuoteQuantity, &t.Commission, &t.CommissionAsset, + &t.ExecutedAt, &t.IsMaker, &t.Metadata, &t.CreatedAt, + ); err != nil { + return nil, err + } + trades = append(trades, t) + } + return trades, rows.Err() +} From ba794d9f3025080120b29798421865128570dd62 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:28:58 +0530 Subject: [PATCH 02/19] feat(api): add GET /api/v1/trades endpoint for fill records Adds TradesHandler in internal/api/trades.go with ListTrades supporting limit/offset query params, and registers it in cmd/api/routes.go after the dashboard handler block. Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/routes.go | 4 +++ internal/api/trades.go | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 internal/api/trades.go diff --git a/cmd/api/routes.go b/cmd/api/routes.go index 5ac451d..8c8b902 100644 --- a/cmd/api/routes.go +++ b/cmd/api/routes.go @@ -236,6 +236,10 @@ func (s *APIServer) setupRoutes() { dashboardHandler := api.NewDashboardHandlerWithOrchestrator(s.db, orchClient, config.Version) dashboardHandler.RegisterRoutesWithRateLimiter(v1, s.rateLimiter.ReadMiddleware(), s.rateLimiter.ControlMiddleware()) + // Trades routes — fill records from the trades table + tradesHandler := api.NewTradesHandler(s.db) + tradesHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware()) + // TB-006: API Key Management routes // These endpoints allow users to manage their API keys (create, rotate, revoke) // All key management operations require authentication diff --git a/internal/api/trades.go b/internal/api/trades.go new file mode 100644 index 0000000..4fbf93c --- /dev/null +++ b/internal/api/trades.go @@ -0,0 +1,57 @@ +package api + +import ( + "net/http" + "strconv" + + "github.com/gin-gonic/gin" + + "github.com/ajitpratap0/cryptofunk/internal/db" +) + +// TradesHandler serves fill records from the trades table. +type TradesHandler struct { + db *db.DB +} + +func NewTradesHandler(database *db.DB) *TradesHandler { + return &TradesHandler{db: database} +} + +func (h *TradesHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc) { + trades := rg.Group("/trades") + trades.Use(readMiddleware) + trades.GET("", h.ListTrades) +} + +// ListTrades returns recent trade fills, newest first. +// GET /api/v1/trades?limit=50&offset=0 +func (h *TradesHandler) ListTrades(c *gin.Context) { + limit := 50 + offset := 0 + if l := c.Query("limit"); l != "" { + if v, err := strconv.Atoi(l); err == nil && v > 0 && v <= 500 { + limit = v + } + } + if o := c.Query("offset"); o != "" { + if v, err := strconv.Atoi(o); err == nil && v >= 0 { + offset = v + } + } + + trades, err := h.db.ListAllTrades(c.Request.Context(), limit, offset) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to fetch trades"}) + return + } + if trades == nil { + trades = []*db.Trade{} + } + c.JSON(http.StatusOK, gin.H{ + "trades": trades, + "count": len(trades), + "limit": limit, + "offset": offset, + }) +} From ffe7404ee8d2711ca999e2c7b223e4f36759e89d Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:29:11 +0530 Subject: [PATCH 03/19] fix(api): handlePaperTrade persists trades, positions, and session stats correctly Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 176 ++++++++++++++++++++++++++++++------ 1 file changed, 149 insertions(+), 27 deletions(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 8d9dff3..a0fcff3 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -1,17 +1,23 @@ package main import ( + "errors" "net/http" "strings" "time" "github.com/gin-gonic/gin" "github.com/google/uuid" + "github.com/jackc/pgx/v5" "github.com/rs/zerolog/log" "github.com/ajitpratap0/cryptofunk/internal/db" + "github.com/ajitpratap0/cryptofunk/internal/exchange" ) +func ptrStr(s string) *string { return &s } +func ptrF64(f float64) *float64 { return &f } + // Session handlers func (s *APIServer) handleListSessions(c *gin.Context) { ctx := c.Request.Context() @@ -389,57 +395,110 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { Quantity float64 `json:"quantity" binding:"required,gt=0"` Price float64 `json:"price"` } - if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "invalid request body", - "details": err.Error(), - }) + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body", "details": err.Error()}) + return + } + isLimit := strings.EqualFold(req.Type, "limit") + if isLimit && req.Price <= 0 { + c.JSON(http.StatusBadRequest, gin.H{"error": "price is required for limit orders"}) return } - if (req.Type == "limit" || req.Type == "LIMIT") && req.Price <= 0 { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "price is required for limit orders", - }) + ctx := c.Request.Context() + + // 1. Resolve or create paper session + sessions, err := s.db.ListActiveSessions(ctx) + if err != nil { + log.Error().Err(err).Msg("Failed to list active sessions for paper trade") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to resolve trading session"}) return } + var sessionID *uuid.UUID + for i := range sessions { + if sessions[i].Mode == db.TradingModePaper { + id := sessions[i].ID + sessionID = &id + break + } + } + if sessionID == nil { + newSession := &db.TradingSession{ + ID: uuid.New(), + Mode: db.TradingModePaper, + Symbol: req.Symbol, + Exchange: "paper", + InitialCapital: 100_000.0, + StartedAt: time.Now(), + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + if err := s.db.CreateSession(ctx, newSession); err != nil { + log.Error().Err(err).Msg("Failed to create paper session") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create trading session"}) + return + } + sessionID = &newSession.ID + } - var price *float64 + // 2. Determine execution price + refPrice := req.Price + if !isLimit && refPrice <= 0 { + mockEx := exchange.NewMockExchange(s.db) + refPrice = mockEx.GetMarketPrice(req.Symbol) + if refPrice <= 0 { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "no market price configured for symbol; provide a price field", + }) + return + } + } + execPrice := refPrice + if !isLimit { + if strings.EqualFold(req.Side, "BUY") { + execPrice = refPrice * 1.001 + } else { + execPrice = refPrice * 0.999 + } + } + + // 3. Insert order + now := time.Now() + var pricePtr *float64 if req.Price > 0 { - price = &req.Price + pricePtr = ptrF64(req.Price) } + orderSide := db.ConvertOrderSide(req.Side) + orderType := db.ConvertOrderType(req.Type) order := &db.Order{ ID: uuid.New(), + SessionID: sessionID, Symbol: req.Symbol, Exchange: "paper", - Side: db.ConvertOrderSide(req.Side), - Type: db.ConvertOrderType(req.Type), + Side: orderSide, + Type: orderType, Quantity: req.Quantity, - Price: price, + Price: pricePtr, Status: db.OrderStatusNew, - PlacedAt: time.Now(), - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + PlacedAt: now, + CreatedAt: now, + UpdatedAt: now, } - - ctx := c.Request.Context() if err := s.db.InsertOrder(ctx, order); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{ - "error": "failed to create paper trade order", - }) + log.Error().Err(err).Msg("Failed to insert paper trade order") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create paper trade order"}) return } - // Simulate immediate fill for market orders + // 4. Immediate fill for market orders if order.Type == db.OrderTypeMarket { - now := time.Now() - execPrice := req.Price execQuoteQty := execPrice * req.Quantity + commission := execQuoteQty * 0.001 - if err := s.db.UpdateOrderStatus(ctx, order.ID, db.OrderStatusFilled, req.Quantity, execQuoteQty, &now, nil, nil); err != nil { - log.Warn().Err(err).Str("order_id", order.ID.String()).Msg("Failed to mark paper trade order as filled") + if err := s.db.UpdateOrderStatus(ctx, order.ID, db.OrderStatusFilled, + req.Quantity, execQuoteQty, &now, nil, nil); err != nil { + log.Warn().Err(err).Str("order_id", order.ID.String()).Msg("Failed to mark paper order filled") } else { order.Status = db.OrderStatusFilled order.ExecutedQuantity = req.Quantity @@ -447,6 +506,69 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { order.FilledAt = &now order.UpdatedAt = now } + + // Write fill record + commissionAsset := "USDT" + trade := &db.Trade{ + ID: uuid.New(), + OrderID: order.ID, + Symbol: req.Symbol, + Exchange: "paper", + Side: orderSide, + Price: execPrice, + Quantity: req.Quantity, + QuoteQuantity: execQuoteQty, + Commission: commission, + CommissionAsset: &commissionAsset, + ExecutedAt: now, + IsMaker: false, + CreatedAt: now, + } + if err := s.db.InsertTrade(ctx, trade); err != nil { + log.Warn().Err(err).Msg("Failed to insert paper trade fill row") + } + + // Create or average into existing position + existingPos, posErr := s.db.GetPositionBySymbolAndSession(ctx, req.Symbol, *sessionID) + if posErr != nil && !errors.Is(posErr, pgx.ErrNoRows) { + log.Warn().Err(posErr).Msg("Error looking up existing position") + } + + posSide := db.PositionSideLong + if orderSide == db.OrderSideSell { + posSide = db.PositionSideShort + } + + if existingPos == nil { + pos := &db.Position{ + ID: uuid.New(), + SessionID: sessionID, + Symbol: req.Symbol, + Exchange: "paper", + Side: posSide, + EntryPrice: execPrice, + Quantity: req.Quantity, + EntryTime: now, + Fees: commission, + EntryReason: ptrStr("paper_trade_api"), + UnrealizedPnL: ptrF64(0), + CreatedAt: now, + UpdatedAt: now, + } + if err := s.db.CreatePosition(ctx, pos); err != nil { + log.Warn().Err(err).Msg("Failed to create position for paper trade") + } + } else { + totalQty := existingPos.Quantity + req.Quantity + weightedAvg := (existingPos.Quantity*existingPos.EntryPrice + req.Quantity*execPrice) / totalQty + if err := s.db.UpdatePositionAveraging(ctx, existingPos.ID, weightedAvg, totalQty, commission); err != nil { + log.Warn().Err(err).Msg("Failed to update position for paper trade") + } + } + + if err := s.db.AggregateSessionStats(ctx, *sessionID); err != nil { + log.Warn().Err(err).Msg("Failed to aggregate session stats after paper trade") + } } if err := s.BroadcastOrderUpdate(order); err != nil { From 2f406f2d7fec67d3beccd09d267e1b61632d51f7 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:34:08 +0530 Subject: [PATCH 04/19] feat: add risk/performance endpoints and fix frontend trade data pipeline - internal/api/risk.go: RiskHandler with /risk/metrics, /risk/circuit-breakers, /risk/exposure endpoints; uses []interface{} conversion for CalculateVaR - internal/api/performance.go: PerformanceHandler with /performance/pairs endpoint aggregating realized PnL by symbol across active sessions - cmd/api/routes.go: register riskHandler and perfHandler in setupRoutes - web/dashboard/lib/api.ts: add getTrades(), getRiskMetrics(), getCircuitBreakers(), getRiskExposure(), getPairPerformance() methods to ApiClient - web/dashboard/hooks/useTradeData.ts: fix useTrades() to hit real API (respects USE_MOCK_DATA), normalize response to Trade[], fix equity curve hardcode to return empty array instead of mock data when using real API Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/routes.go | 8 ++ internal/api/performance.go | 87 ++++++++++++ internal/api/risk.go | 199 ++++++++++++++++++++++++++++ web/dashboard/hooks/useTradeData.ts | 31 ++++- web/dashboard/lib/api.ts | 39 ++++++ 5 files changed, 357 insertions(+), 7 deletions(-) create mode 100644 internal/api/performance.go create mode 100644 internal/api/risk.go diff --git a/cmd/api/routes.go b/cmd/api/routes.go index 8c8b902..8f1736f 100644 --- a/cmd/api/routes.go +++ b/cmd/api/routes.go @@ -264,6 +264,14 @@ func (s *APIServer) setupRoutes() { polymarketHandler := api.NewPolymarketHandler(s.db) polymarketHandler.RegisterRoutesWithRateLimiter(v1, s.rateLimiter.ReadMiddleware(), s.rateLimiter.OrderMiddleware()) + // Risk metrics routes + riskHandler := api.NewRiskHandler(s.db) + riskHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware()) + + // Performance routes + perfHandler := api.NewPerformanceHandler(s.db) + perfHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware()) + // Decision analytics and outcome resolution routes decisionAnalyticsHandler := api.NewDecisionAnalyticsHandler(s.db) decisionAnalyticsHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware(), api.AuthMiddleware(s.apiKeyStore, authConfig)) diff --git a/internal/api/performance.go b/internal/api/performance.go new file mode 100644 index 0000000..5ac536e --- /dev/null +++ b/internal/api/performance.go @@ -0,0 +1,87 @@ +package api + +import ( + "context" + "math" + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/ajitpratap0/cryptofunk/internal/db" +) + +// PerformanceHandler provides REST endpoints for per-symbol performance metrics. +type PerformanceHandler struct { + db *db.DB +} + +// NewPerformanceHandler creates a new PerformanceHandler backed by the given database. +func NewPerformanceHandler(database *db.DB) *PerformanceHandler { + return &PerformanceHandler{db: database} +} + +// RegisterRoutes mounts the /performance sub-group under the provided router group. +func (h *PerformanceHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc) { + g := rg.Group("/performance") + g.Use(readMiddleware) + g.GET("/pairs", h.GetPairPerformance) +} + +// GetPairPerformance returns realized PnL aggregated by trading pair across all active sessions. +// GET /api/v1/performance/pairs +func (h *PerformanceHandler) GetPairPerformance(c *gin.Context) { + ctx := c.Request.Context() + + pairs, err := h.aggregatePairPerformance(ctx) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to aggregate pair performance"}) + return + } + + c.JSON(http.StatusOK, gin.H{"pairs": pairs, "count": len(pairs)}) +} + +type pairPerf struct { + Symbol string `json:"symbol"` + RealizedPnL float64 `json:"realized_pnl"` + TradeCount int `json:"trade_count"` +} + +func (h *PerformanceHandler) aggregatePairPerformance(ctx context.Context) ([]pairPerf, error) { + sessions, err := h.db.ListActiveSessions(ctx) + if err != nil { + return nil, err + } + + type agg struct { + pnl float64 + count int + } + bySymbol := make(map[string]*agg) + + for _, s := range sessions { + positions, err := h.db.GetPositionsBySession(ctx, s.ID) + if err != nil { + continue + } + for _, p := range positions { + if p.ExitTime != nil && p.RealizedPnL != nil { + if _, ok := bySymbol[p.Symbol]; !ok { + bySymbol[p.Symbol] = &agg{} + } + bySymbol[p.Symbol].pnl += *p.RealizedPnL + bySymbol[p.Symbol].count++ + } + } + } + + result := make([]pairPerf, 0, len(bySymbol)) + for sym, a := range bySymbol { + result = append(result, pairPerf{ + Symbol: sym, + RealizedPnL: math.Round(a.pnl*100) / 100, + TradeCount: a.count, + }) + } + return result, nil +} diff --git a/internal/api/risk.go b/internal/api/risk.go new file mode 100644 index 0000000..6f0f572 --- /dev/null +++ b/internal/api/risk.go @@ -0,0 +1,199 @@ +package api + +import ( + "context" + "math" + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/ajitpratap0/cryptofunk/internal/db" + "github.com/ajitpratap0/cryptofunk/internal/risk" +) + +// RiskHandler provides REST endpoints for risk metrics and circuit breaker status. +type RiskHandler struct { + db *db.DB + riskService *risk.Service +} + +// NewRiskHandler creates a new RiskHandler backed by the given database. +func NewRiskHandler(database *db.DB) *RiskHandler { + return &RiskHandler{ + db: database, + riskService: risk.NewService(), + } +} + +// RegisterRoutes mounts the /risk sub-group under the provided router group. +func (h *RiskHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc) { + r := rg.Group("/risk") + r.Use(readMiddleware) + r.GET("/metrics", h.GetMetrics) + r.GET("/circuit-breakers", h.GetCircuitBreakers) + r.GET("/exposure", h.GetExposure) +} + +// GetMetrics returns VaR, CVaR, open position count, and total exposure. +// GET /api/v1/risk/metrics +func (h *RiskHandler) GetMetrics(c *gin.Context) { + ctx := c.Request.Context() + + openPositions, err := h.db.GetAllOpenPositions(ctx) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to query open positions"}) + return + } + + openCount := len(openPositions) + var totalExposure float64 + for _, p := range openPositions { + totalExposure += p.Quantity * p.EntryPrice + } + + returns, err := h.collectClosedReturns(ctx) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to collect returns"}) + return + } + + response := gin.H{ + "open_positions": openCount, + "total_exposure": math.Round(totalExposure*100) / 100, + "data_points": len(returns), + "var_95": nil, + "var_99": nil, + "expected_shortfall": nil, + } + + if len(returns) >= 10 { + // CalculateVaR requires []interface{} not []float64 + returnsIface := make([]interface{}, len(returns)) + for i, v := range returns { + returnsIface[i] = v + } + + res95, err := h.riskService.CalculateVaR(map[string]interface{}{ + "returns": returnsIface, + "confidence_level": 0.95, + }) + if err == nil { + if varResult, ok := res95.(*risk.VaRResult); ok { + response["var_95"] = varResult.VaR + } + } + + res99, err := h.riskService.CalculateVaR(map[string]interface{}{ + "returns": returnsIface, + "confidence_level": 0.99, + }) + if err == nil { + if varResult, ok := res99.(*risk.VaRResult); ok { + response["var_99"] = varResult.VaR + response["expected_shortfall"] = varResult.CVaR + } + } + } + + c.JSON(http.StatusOK, response) +} + +// GetCircuitBreakers returns the status of system-level circuit breakers. +// GET /api/v1/risk/circuit-breakers +func (h *RiskHandler) GetCircuitBreakers(c *gin.Context) { + ctx := c.Request.Context() + + sessions, err := h.db.ListActiveSessions(ctx) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list sessions"}) + return + } + + var totalPnL, maxDrawdown float64 + var totalTrades int + for _, s := range sessions { + totalPnL += s.TotalPnL + if s.MaxDrawdown > maxDrawdown { + maxDrawdown = s.MaxDrawdown + } + totalTrades += s.TotalTrades + } + + // Max Daily Loss: triggered when cumulative losses exceed $5000 + lossAmount := math.Abs(math.Min(totalPnL, 0)) + breakers := []gin.H{ + buildBreaker("Max Daily Loss", lossAmount, 5000), + buildBreaker("Max Drawdown %", maxDrawdown*100, 10), + buildBreaker("Total Trade Count", float64(totalTrades), 100), + } + + c.JSON(http.StatusOK, gin.H{"circuit_breakers": breakers, "count": len(breakers)}) +} + +func buildBreaker(name string, current, threshold float64) gin.H { + status := "OK" + if current >= threshold { + status = "TRIGGERED" + } else if threshold > 0 && current/threshold >= 0.8 { + status = "WARNING" + } + return gin.H{ + "name": name, + "current": math.Round(current*100) / 100, + "threshold": threshold, + "status": status, + } +} + +// GetExposure returns open position exposure grouped by symbol. +// GET /api/v1/risk/exposure +func (h *RiskHandler) GetExposure(c *gin.Context) { + ctx := c.Request.Context() + + openPositions, err := h.db.GetAllOpenPositions(ctx) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to query open positions"}) + return + } + + exposureBySymbol := make(map[string]float64) + for _, p := range openPositions { + exposureBySymbol[p.Symbol] += p.Quantity * p.EntryPrice + } + + type symbolExposure struct { + Symbol string `json:"symbol"` + Exposure float64 `json:"exposure"` + } + result := make([]symbolExposure, 0, len(exposureBySymbol)) + for sym, exp := range exposureBySymbol { + result = append(result, symbolExposure{ + Symbol: sym, + Exposure: math.Round(exp*100) / 100, + }) + } + + c.JSON(http.StatusOK, gin.H{"exposure": result, "count": len(result)}) +} + +// collectClosedReturns gathers RealizedPnL from closed positions across active sessions. +func (h *RiskHandler) collectClosedReturns(ctx context.Context) ([]float64, error) { + sessions, err := h.db.ListActiveSessions(ctx) + if err != nil { + return nil, err + } + + var returns []float64 + for _, s := range sessions { + positions, err := h.db.GetPositionsBySession(ctx, s.ID) + if err != nil { + continue + } + for _, p := range positions { + if p.ExitTime != nil && p.RealizedPnL != nil { + returns = append(returns, *p.RealizedPnL) + } + } + } + return returns, nil +} diff --git a/web/dashboard/hooks/useTradeData.ts b/web/dashboard/hooks/useTradeData.ts index bc87aa1..723da53 100644 --- a/web/dashboard/hooks/useTradeData.ts +++ b/web/dashboard/hooks/useTradeData.ts @@ -18,7 +18,7 @@ import { isRawDashboardResponse, isRawDashboardPnlResponse, } from '@/types/api-responses' -import type { Trade, Position, Order, UnifiedPortfolio, DashboardStats } from '@/lib/types' +import type { Trade, Position, Order, UnifiedPortfolio, DashboardStats, EquityPoint } from '@/lib/types' // Query Keys export const QUERY_KEYS = { @@ -33,15 +33,32 @@ export const QUERY_KEYS = { } as const // Trades -export function useTrades() { +export function useTrades(limit = 50, offset = 0) { return useQuery({ - queryKey: [...QUERY_KEYS.trades], + queryKey: [...QUERY_KEYS.trades, limit, offset], queryFn: async () => { - const mockTrades = getMockTrades() + if (USE_MOCK_DATA) { + return { + success: true as const, + data: getMockTrades(), + timestamp: new Date().toISOString(), + } + } + + const response = await apiClient.getTrades(limit, offset) + if (!response.success) { + throw new Error(response.error || 'Failed to fetch trades') + } + + const raw: unknown = response.data + const tradeList: Trade[] = + raw && typeof raw === 'object' && 'trades' in raw + ? (raw as { trades: Trade[] }).trades + : [] return { success: true as const, - data: mockTrades, - timestamp: new Date().toISOString(), + data: tradeList, + timestamp: response.timestamp, } }, staleTime: REFRESH_INTERVALS.trades, @@ -262,7 +279,7 @@ export function useDashboardPnl() { data: { daily: raw.realized_pnl ?? 0, total: raw.total_pnl ?? 0, - equity: getMockEquityPoints(), // No equity curve from API yet + equity: (raw as { equity_curve?: EquityPoint[] }).equity_curve ?? [], }, timestamp: response.timestamp, } diff --git a/web/dashboard/lib/api.ts b/web/dashboard/lib/api.ts index 986c3b8..105436e 100644 --- a/web/dashboard/lib/api.ts +++ b/web/dashboard/lib/api.ts @@ -276,6 +276,45 @@ class ApiClient { body: JSON.stringify(trade), }) } + + // Trades (fill records) + async getTrades(limit = 50, offset = 0): Promise> { + return this.request(`/trades?limit=${limit}&offset=${offset}`) + } + + // Risk metrics + async getRiskMetrics(): Promise> { + return this.request('/risk/metrics') + } + + async getCircuitBreakers(): Promise + count: number + }>> { + return this.request('/risk/circuit-breakers') + } + + async getRiskExposure(): Promise + count: number + }>> { + return this.request('/risk/exposure') + } + + // Performance by trading pair + async getPairPerformance(): Promise + count: number + }>> { + return this.request('/performance/pairs') + } } export const apiClient = new ApiClient() From 3cf27ea31dd17635b0ab3ea5ff5acbe3e6e4cab0 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:38:37 +0530 Subject: [PATCH 05/19] feat(dashboard): wire risk page and performance hooks to real API - hooks/usePerformance.ts: fix usePairPerformance, useRiskMetrics, useCircuitBreakers to use apiClient (proper /api/v1 prefix); add useRiskExposure hook; remove stale RiskMetrics/CircuitBreaker imports - app/risk/page.tsx: replace all hardcoded mock constants with real API data via useRiskMetrics/useCircuitBreakers/useRiskExposure hooks; map backend status strings (OK/WARNING/TRIGGERED) to component shape; compute exposure percentages from API totals; keep drawdown/alerts as static placeholders (no API equivalent yet) Co-Authored-By: Claude Sonnet 4.6 --- web/dashboard/app/risk/page.tsx | 263 ++++++++++---------------- web/dashboard/hooks/usePerformance.ts | 132 +++---------- 2 files changed, 126 insertions(+), 269 deletions(-) diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index f69cde2..5768373 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -1,13 +1,11 @@ 'use client' import { useState, useMemo } from 'react' -import { - Shield, - AlertTriangle, - TrendingDown, - Activity, +import { + Shield, + TrendingDown, + Activity, RefreshCw, - ChevronDown, BarChart3, Zap } from 'lucide-react' @@ -16,80 +14,21 @@ import { DrawdownChart } from '@/components/charts/DrawdownChart' import { CircuitBreakerStatus } from '@/components/risk/CircuitBreakerStatus' import { StatCard } from '@/components/ui/StatCard' import { cn, formatCurrency, formatPercentage } from '@/lib/utils' -import { COLORS, RISK_THRESHOLDS } from '@/lib/constants' -import type { CircuitBreaker, RiskMetrics } from '@/lib/types' +import { RISK_THRESHOLDS } from '@/lib/constants' +import { useRiskMetrics, useCircuitBreakers, useRiskExposure } from '@/hooks/usePerformance' +import type { CircuitBreaker } from '@/lib/types' -// ── Mock Data ────────────────────────────────────────────────────── - -const mockRiskMetrics: RiskMetrics = { - var95: 0.023, - var99: 0.041, - expectedShortfall: 0.052, - betaToMarket: 0.87, - correlation: { - 'BTC/USDT': 1.0, - 'ETH/USDT': 0.89, - 'SOL/USDT': 0.76, - 'XRP/USDT': 0.64, - 'ADA/USDT': 0.58, - }, - concentration: { - 'BTC/USDT': 0.35, - 'ETH/USDT': 0.25, - 'SOL/USDT': 0.18, - 'XRP/USDT': 0.12, - 'ADA/USDT': 0.10, - }, -} - -const mockCircuitBreakers: CircuitBreaker[] = [ - { - name: 'Max Daily Loss', - status: 'normal', - threshold: 0.05, - currentValue: 0.018, - description: 'Maximum allowed daily portfolio loss', - }, - { - name: 'Max Drawdown', - status: 'warning', - threshold: 0.10, - currentValue: 0.078, - description: 'Maximum peak-to-trough drawdown', - }, - { - name: 'Position Concentration', - status: 'normal', - threshold: 0.50, - currentValue: 0.35, - description: 'Maximum single-asset concentration', - }, - { - name: 'Leverage Limit', - status: 'normal', - threshold: 10, - currentValue: 3.2, - description: 'Maximum portfolio leverage ratio', - }, -] - -const mockExposure = [ - { symbol: 'BTC/USDT', exposure: 35000, percentage: 35, value: 35000, side: 'long' as const }, - { symbol: 'ETH/USDT', exposure: 25000, percentage: 25, value: 25000, side: 'long' as const }, - { symbol: 'SOL/USDT', exposure: 18000, percentage: 18, value: 18000, side: 'long' as const }, - { symbol: 'XRP/USDT', exposure: 12000, percentage: 12, value: 12000, side: 'short' as const }, - { symbol: 'ADA/USDT', exposure: 10000, percentage: 10, value: 10000, side: 'long' as const }, -] +// ── Static Mock Data (no API equivalent yet) ─────────────────────── const mockDrawdown = Array.from({ length: 90 }, (_, i) => { const now = Date.now() const timestamp = new Date(now - (89 - i) * 24 * 60 * 60 * 1000).toISOString() - const base = Math.sin(i / 15) * 4 + Math.random() * 2 // Generate percentage values (0-6%) - const drawdownPercent = -Math.abs(base) // Negative percentage value (-6% to 0%) + const base = Math.sin(i / 15) * 4 + Math.random() * 2 + const drawdownPercent = -Math.abs(base) return { timestamp, drawdown: drawdownPercent, - equity: 100000 + (drawdownPercent / 100) * 100000, // Convert percentage to equity change + equity: 100000 + (drawdownPercent / 100) * 100000, } }) @@ -106,16 +45,70 @@ const mockAlerts = [ export default function RiskPage() { const [timeRange, setTimeRange] = useState<'1w' | '1m' | '3m'>('1m') + const { data: metricsResponse, refetch: refetchMetrics } = useRiskMetrics() + const { data: breakersResponse, refetch: refetchBreakers } = useCircuitBreakers() + const { data: exposureResponse, refetch: refetchExposure } = useRiskExposure() + + const handleRefresh = () => { + refetchMetrics() + refetchBreakers() + refetchExposure() + } + + // Map API risk metrics + const apiMetrics = metricsResponse?.data as { + var_95: number | null + var_99: number | null + expected_shortfall: number | null + open_positions: number + total_exposure: number + } | undefined + + // Map API circuit breakers to component shape + const circuitBreakers: CircuitBreaker[] = useMemo(() => { + const raw = breakersResponse?.data as + | { circuit_breakers: Array<{ name: string; current: number; threshold: number; status: string }> } + | undefined + if (!raw?.circuit_breakers?.length) return [] + return raw.circuit_breakers.map(cb => ({ + name: cb.name, + status: cb.status === 'TRIGGERED' ? 'triggered' : cb.status === 'WARNING' ? 'warning' : 'normal', + threshold: cb.threshold, + currentValue: cb.current, + description: cb.name, + } as CircuitBreaker)) + }, [breakersResponse]) + + // Map API exposure data + const exposureData = useMemo(() => { + const raw = exposureResponse?.data as + | { exposure: Array<{ symbol: string; exposure: number }> } + | undefined + if (!raw?.exposure?.length) return [] + const total = raw.exposure.reduce((sum, e) => sum + e.exposure, 0) + return raw.exposure.map(e => ({ + symbol: e.symbol, + exposure: e.exposure, + percentage: total > 0 ? Math.round((e.exposure / total) * 100) : 0, + value: e.exposure, + side: 'long' as const, + })) + }, [exposureResponse]) + + const var95 = apiMetrics?.var_95 ?? null + const var99 = apiMetrics?.var_99 ?? null + const riskScore = useMemo(() => { + if (!circuitBreakers.length) return 100 let score = 100 - mockCircuitBreakers.forEach(cb => { - const usage = Math.abs(cb.currentValue) / Math.abs(cb.threshold) + circuitBreakers.forEach(cb => { + const usage = cb.threshold > 0 ? Math.abs(cb.currentValue) / Math.abs(cb.threshold) : 0 if (cb.status === 'triggered') score -= 30 else if (cb.status === 'warning') score -= 15 else if (usage > 0.5) score -= 5 }) return Math.max(0, Math.min(100, score)) - }, []) + }, [circuitBreakers]) const riskLevel = riskScore >= 80 ? 'Low' : riskScore >= 50 ? 'Medium' : 'High' const riskColor = riskScore >= 80 ? 'text-profit' : riskScore >= 50 ? 'text-warning' : 'text-loss' @@ -130,7 +123,10 @@ export default function RiskPage() { Portfolio risk monitoring & circuit breaker status

- @@ -147,22 +143,21 @@ export default function RiskPage() { /> } - valueClassName={mockRiskMetrics.var95 > RISK_THRESHOLDS.var.warning ? 'text-warning' : 'text-foreground'} + valueClassName={var95 != null && var95 > RISK_THRESHOLDS.var.warning ? 'text-warning' : 'text-foreground'} /> } - valueClassName="text-warning" /> } /> @@ -171,7 +166,7 @@ export default function RiskPage() {
{/* Left: Circuit Breakers + Alerts */}
- + {/* Risk Alerts */}
@@ -216,7 +211,7 @@ export default function RiskPage() { {/* Exposure Pie */}
- {/* Correlation Matrix */} -
-

Asset Correlation Matrix

-
- - - - - {Object.keys(mockRiskMetrics.correlation).map(asset => ( - - ))} - - - - {Object.entries(mockRiskMetrics.correlation).map(([asset, _]) => ( - - - {Object.keys(mockRiskMetrics.correlation).map(otherAsset => { - const corr = asset === otherAsset - ? 1.0 - : Math.min( - (mockRiskMetrics.correlation[asset] || 0.5) * - (mockRiskMetrics.correlation[otherAsset] || 0.5), - 0.99 - ) - const intensity = Math.abs(corr) - return ( - - ) - })} - - ))} - -
Asset - {asset.replace('/USDT', '')} -
{asset.replace('/USDT', '')} 0 ? '34,197,94' : '239,68,68'}, ${intensity * 0.3})`, - }} - > - {corr.toFixed(2)} -
-
-
- {/* Position Sizing */} -
-

Position Sizing Breakdown

-
- {mockExposure.map(pos => ( -
-
{pos.symbol.replace('/USDT', '')}
-
-
-
+ {exposureData.length > 0 && ( +
+

Position Sizing Breakdown

+
+ {exposureData.map(pos => ( +
+
{pos.symbol.replace('/USDT', '').replace('USDT', '')}
+
+
+
+
+
{formatCurrency(pos.value)}
+
{pos.percentage}%
-
{formatCurrency(pos.value)}
-
{pos.percentage}%
- - {pos.side} - -
- ))} + ))} +
-
+ )}
diff --git a/web/dashboard/hooks/usePerformance.ts b/web/dashboard/hooks/usePerformance.ts index f74ca3e..cf9a9eb 100644 --- a/web/dashboard/hooks/usePerformance.ts +++ b/web/dashboard/hooks/usePerformance.ts @@ -9,14 +9,12 @@ import { calculateWinRate, groupBy } from '@/lib/utils' -import type { - PerformanceMetrics, - EquityPoint, - CandlestickData, +import type { + PerformanceMetrics, + EquityPoint, + CandlestickData, TimeRange, Trade, - RiskMetrics, - CircuitBreaker } from '@/lib/types' // Query Keys @@ -174,28 +172,14 @@ export function usePairPerformance() { return useQuery({ queryKey: PERFORMANCE_QUERY_KEYS.pairPerformance, queryFn: async () => { - try { - // Try to get actual data from API - const response = await fetch(`${process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8080'}/performance/pairs`) - if (!response.ok) throw new Error('API error') - return await response.json() - } catch { - // Return mock data if API fails - const mockPairData = [ - { pair: 'BTC/USDT', pnl: 4520.30, trades: 45, winRate: 72.5, avgReturn: 2.8 }, - { pair: 'ETH/USDT', pnl: 3240.15, trades: 38, winRate: 68.4, avgReturn: 3.1 }, - { pair: 'BNB/USDT', pnl: 1890.45, trades: 52, winRate: 65.2, avgReturn: 1.9 }, - { pair: 'XRP/USDT', pnl: -234.56, trades: 23, winRate: 43.5, avgReturn: -0.5 }, - { pair: 'ADA/USDT', pnl: 567.89, trades: 31, winRate: 58.1, avgReturn: 1.2 }, - { pair: 'SOL/USDT', pnl: 2134.67, trades: 28, winRate: 75.0, avgReturn: 4.2 }, - ] - - return { - success: true, - data: mockPairData, - timestamp: new Date().toISOString(), - } - } + const response = await apiClient.getPairPerformance() + if (!response.success) throw new Error(response.error || 'Failed to fetch pair performance') + const raw: unknown = response.data + const pairs = + raw && typeof raw === 'object' && 'pairs' in raw + ? (raw as { pairs: Array<{ symbol: string; realized_pnl: number; trade_count: number }> }).pairs + : [] + return { success: true as const, data: pairs, timestamp: response.timestamp } }, staleTime: REFRESH_INTERVALS.performance, refetchInterval: REFRESH_INTERVALS.performance, @@ -235,40 +219,7 @@ export function useCandlestickData(symbol: string, timeRange: string = '1d') { export function useRiskMetrics() { return useQuery({ queryKey: PERFORMANCE_QUERY_KEYS.risk, - queryFn: async () => { - try { - // Try to get actual risk data - const response = await fetch(`${process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8080'}/risk/metrics`) - if (!response.ok) throw new Error('API error') - return await response.json() - } catch { - // Return mock data if API fails - const mockRiskMetrics: RiskMetrics = { - var95: 0.025, // 2.5% - var99: 0.045, // 4.5% - expectedShortfall: 0.062, // 6.2% - betaToMarket: 1.23, - correlation: { - 'BTC/USDT': 0.85, - 'ETH/USDT': 0.78, - 'BNB/USDT': 0.65, - 'Market': 1.0, - }, - concentration: { - 'BTC/USDT': 0.35, - 'ETH/USDT': 0.25, - 'BNB/USDT': 0.15, - 'Others': 0.25, - }, - } - - return { - success: true, - data: mockRiskMetrics, - timestamp: new Date().toISOString(), - } - } - }, + queryFn: () => apiClient.getRiskMetrics(), staleTime: REFRESH_INTERVALS.risk, refetchInterval: REFRESH_INTERVALS.risk, }) @@ -278,52 +229,17 @@ export function useRiskMetrics() { export function useCircuitBreakers() { return useQuery({ queryKey: PERFORMANCE_QUERY_KEYS.circuitBreakers, - queryFn: async () => { - try { - // Try to get actual circuit breaker data - const response = await fetch(`${process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8080'}/risk/circuit-breakers`) - if (!response.ok) throw new Error('API error') - return await response.json() - } catch { - // Return mock data if API fails - const mockCircuitBreakers: CircuitBreaker[] = [ - { - name: 'Max Daily Loss', - status: 'normal', - threshold: 5.0, - currentValue: 2.3, - description: 'Maximum daily loss percentage', - }, - { - name: 'Max Drawdown', - status: 'warning', - threshold: 10.0, - currentValue: 8.5, - description: 'Maximum portfolio drawdown', - }, - { - name: 'Position Concentration', - status: 'normal', - threshold: 50.0, - currentValue: 35.0, - description: 'Maximum single position exposure', - }, - { - name: 'Leverage Limit', - status: 'normal', - threshold: 10.0, - currentValue: 6.2, - description: 'Maximum portfolio leverage', - }, - ] - - return { - success: true, - data: mockCircuitBreakers, - timestamp: new Date().toISOString(), - } - } - }, + queryFn: () => apiClient.getCircuitBreakers(), + staleTime: REFRESH_INTERVALS.risk, + refetchInterval: REFRESH_INTERVALS.risk, + }) +} + +// Risk Exposure by symbol +export function useRiskExposure() { + return useQuery({ + queryKey: [...PERFORMANCE_QUERY_KEYS.risk, 'exposure'], + queryFn: () => apiClient.getRiskExposure(), staleTime: REFRESH_INTERVALS.risk, refetchInterval: REFRESH_INTERVALS.risk, }) From 4d56296fef23d92cb3dd036adc60db06ac2f11d0 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:44:48 +0530 Subject: [PATCH 06/19] test(dashboard): add useTrades hook unit tests 4 tests verify the hook hits the real API when USE_MOCK_DATA=false: - returns Trade[] from API response wrapper - throws on API error (success:false) - returns empty array when response has unexpected shape - passes custom limit/offset to apiClient.getTrades Co-Authored-By: Claude Sonnet 4.6 --- web/dashboard/__tests__/useTradeData.test.tsx | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 web/dashboard/__tests__/useTradeData.test.tsx diff --git a/web/dashboard/__tests__/useTradeData.test.tsx b/web/dashboard/__tests__/useTradeData.test.tsx new file mode 100644 index 0000000..649e814 --- /dev/null +++ b/web/dashboard/__tests__/useTradeData.test.tsx @@ -0,0 +1,125 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import type { ReactNode } from 'react' +import { useTrades } from '@/hooks/useTradeData' +import type { Trade } from '@/lib/types' + +// ── Mock apiClient ────────────────────────────────────────────────── + +vi.mock('@/lib/api', () => ({ + apiClient: { + getTrades: vi.fn(), + }, +})) + +// ── Mock mock-data module so USE_MOCK_DATA = false ────────────────── + +vi.mock('@/lib/mock-data', () => ({ + USE_MOCK_DATA: false, + getMockTrades: vi.fn(() => []), + getMockDashboard: vi.fn(() => ({})), + getMockEquityPoints: vi.fn(() => []), + getMockOrders: vi.fn(() => []), + getMockPositionsFromTrades: vi.fn(() => []), + getMockUnifiedPortfolio: vi.fn(() => ({})), +})) + +// ── Test helpers ──────────────────────────────────────────────────── + +function makeWrapper() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }) + return ({ children }: { children: ReactNode }) => ( + {children} + ) +} + +const fakeTrade: Trade = { + id: 'trade-abc', + symbol: 'BTCUSDT', + side: 'long', + entryPrice: 45045, + currentPrice: 45045, + quantity: 0.1, + pnl: 0, + pnlPercent: 0, + agent: 'paper', + confidence: 1, + timestamp: '2026-03-19T16:00:00Z', + status: 'open', +} + +// ── Tests ─────────────────────────────────────────────────────────── + +describe('useTrades', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns trade array from real API when USE_MOCK_DATA is false', async () => { + const { apiClient } = await import('@/lib/api') + vi.mocked(apiClient.getTrades).mockResolvedValueOnce({ + success: true, + data: { trades: [fakeTrade], count: 1 } as unknown as { trades: Trade[]; count: number }, + timestamp: new Date().toISOString(), + }) + + const { result } = renderHook(() => useTrades(), { wrapper: makeWrapper() }) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + expect(apiClient.getTrades).toHaveBeenCalledOnce() + expect(apiClient.getTrades).toHaveBeenCalledWith(50, 0) + expect(result.current.data?.data).toHaveLength(1) + expect(result.current.data?.data[0].symbol).toBe('BTCUSDT') + expect(result.current.data?.data[0].entryPrice).toBe(45045) + }) + + it('throws when API returns success:false', async () => { + const { apiClient } = await import('@/lib/api') + vi.mocked(apiClient.getTrades).mockResolvedValueOnce({ + success: false, + data: null as unknown as { trades: Trade[]; count: number }, + error: 'db error', + timestamp: new Date().toISOString(), + }) + + const { result } = renderHook(() => useTrades(), { wrapper: makeWrapper() }) + + await waitFor(() => expect(result.current.isError).toBe(true)) + + expect((result.current.error as Error).message).toBe('db error') + }) + + it('returns empty array when API response has no trades wrapper', async () => { + const { apiClient } = await import('@/lib/api') + vi.mocked(apiClient.getTrades).mockResolvedValueOnce({ + success: true, + data: null as unknown as { trades: Trade[]; count: number }, + timestamp: new Date().toISOString(), + }) + + const { result } = renderHook(() => useTrades(), { wrapper: makeWrapper() }) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + expect(result.current.data?.data).toEqual([]) + }) + + it('accepts custom limit and offset params', async () => { + const { apiClient } = await import('@/lib/api') + vi.mocked(apiClient.getTrades).mockResolvedValueOnce({ + success: true, + data: { trades: [], count: 0 } as unknown as { trades: Trade[]; count: number }, + timestamp: new Date().toISOString(), + }) + + renderHook(() => useTrades(10, 20), { wrapper: makeWrapper() }) + + await waitFor(() => expect(vi.mocked(apiClient.getTrades)).toHaveBeenCalled()) + + expect(apiClient.getTrades).toHaveBeenCalledWith(10, 20) + }) +}) From e5b42731bc5fa45dfe898a962cb6a1fb1d690a96 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:55:40 +0530 Subject: [PATCH 07/19] fix(lint): apply gofmt formatting to handlers_trading.go --- cmd/api/handlers_trading.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index a0fcff3..5eef3df 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -15,7 +15,7 @@ import ( "github.com/ajitpratap0/cryptofunk/internal/exchange" ) -func ptrStr(s string) *string { return &s } +func ptrStr(s string) *string { return &s } func ptrF64(f float64) *float64 { return &f } // Session handlers From 414a118a178c6aab39cfa79eb569c4445dd72df3 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 21:59:14 +0530 Subject: [PATCH 08/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20code=20r?= =?UTF-8?q?eview=20=E2=80=94=20TOCTOU=20guard,=20N+1=20fix,=20position=20s?= =?UTF-8?q?ide=20warning,=20pagination=20total?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handlers_trading.go: fix TOCTOU race on paper session creation; if CreateSession fails (duplicate key from concurrent request), retry ListActiveSessions and reuse the existing paper session rather than returning 500 - handlers_trading.go: add zerolog Warn when a trade arrives on the opposite side of an existing open position (position close/reduce logic not yet implemented — TODO) - internal/db/positions.go: add GetAllClosedPositions() — single query across all sessions returning positions where exit_time IS NOT NULL - internal/api/risk.go: replace N+1 (ListActiveSessions + GetPositionsBySession loop) in collectClosedReturns with a single GetAllClosedPositions call - internal/api/performance.go: replace N+1 (ListActiveSessions + GetPositionsBySession loop) in aggregatePairPerformance with a single GetAllClosedPositions call - internal/db/trades.go: add CountAllTrades() helper - internal/api/trades.go: add total field to ListTrades response using CountAllTrades; failure is non-fatal (returns -1) so the page data is never blocked Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 37 +++++++++++++++++++++++++++++++++---- internal/api/performance.go | 21 ++++++++------------- internal/api/risk.go | 16 +++++----------- internal/api/trades.go | 11 ++++++++++- internal/db/positions.go | 23 +++++++++++++++++++++++ internal/db/trades.go | 10 ++++++++++ 6 files changed, 89 insertions(+), 29 deletions(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 5eef3df..fa6c61f 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -434,11 +434,27 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { UpdatedAt: time.Now(), } if err := s.db.CreateSession(ctx, newSession); err != nil { - log.Error().Err(err).Msg("Failed to create paper session") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create trading session"}) - return + // Another concurrent request may have created a paper session between the + // ListActiveSessions call above and this insert (TOCTOU). Try to look it up. + log.Warn().Err(err).Msg("Failed to create paper session; retrying lookup for concurrent session") + sessions2, err2 := s.db.ListActiveSessions(ctx) + if err2 == nil { + for i := range sessions2 { + if sessions2[i].Mode == db.TradingModePaper { + id := sessions2[i].ID + sessionID = &id + break + } + } + } + if sessionID == nil { + log.Error().Err(err).Msg("Failed to create or find paper session") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create trading session"}) + return + } + } else { + sessionID = &newSession.ID } - sessionID = &newSession.ID } // 2. Determine execution price @@ -539,6 +555,19 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { posSide = db.PositionSideShort } + if existingPos != nil && existingPos.Side != posSide { + // Opposite-side trade on an existing open position. Proper close/reduce + // logic (netting, realized PnL calculation) is not yet implemented. + // For now we average into the opposite direction, which is incorrect for + // a long→short flip. This is a known limitation. + // TODO: implement position close/reduce logic. + log.Warn(). + Str("symbol", req.Symbol). + Str("existing_side", string(existingPos.Side)). + Str("order_side", string(posSide)). + Msg("Opposite-side trade on existing position; position close logic not yet implemented") + } + if existingPos == nil { pos := &db.Position{ ID: uuid.New(), diff --git a/internal/api/performance.go b/internal/api/performance.go index 5ac536e..605608a 100644 --- a/internal/api/performance.go +++ b/internal/api/performance.go @@ -48,7 +48,8 @@ type pairPerf struct { } func (h *PerformanceHandler) aggregatePairPerformance(ctx context.Context) ([]pairPerf, error) { - sessions, err := h.db.ListActiveSessions(ctx) + // Fetch all closed positions in one query to avoid N+1 per session. + positions, err := h.db.GetAllClosedPositions(ctx) if err != nil { return nil, err } @@ -59,19 +60,13 @@ func (h *PerformanceHandler) aggregatePairPerformance(ctx context.Context) ([]pa } bySymbol := make(map[string]*agg) - for _, s := range sessions { - positions, err := h.db.GetPositionsBySession(ctx, s.ID) - if err != nil { - continue - } - for _, p := range positions { - if p.ExitTime != nil && p.RealizedPnL != nil { - if _, ok := bySymbol[p.Symbol]; !ok { - bySymbol[p.Symbol] = &agg{} - } - bySymbol[p.Symbol].pnl += *p.RealizedPnL - bySymbol[p.Symbol].count++ + for _, p := range positions { + if p.RealizedPnL != nil { + if _, ok := bySymbol[p.Symbol]; !ok { + bySymbol[p.Symbol] = &agg{} } + bySymbol[p.Symbol].pnl += *p.RealizedPnL + bySymbol[p.Symbol].count++ } } diff --git a/internal/api/risk.go b/internal/api/risk.go index 6f0f572..c66b734 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -176,23 +176,17 @@ func (h *RiskHandler) GetExposure(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"exposure": result, "count": len(result)}) } -// collectClosedReturns gathers RealizedPnL from closed positions across active sessions. +// collectClosedReturns gathers RealizedPnL from all closed positions in a single query. func (h *RiskHandler) collectClosedReturns(ctx context.Context) ([]float64, error) { - sessions, err := h.db.ListActiveSessions(ctx) + positions, err := h.db.GetAllClosedPositions(ctx) if err != nil { return nil, err } var returns []float64 - for _, s := range sessions { - positions, err := h.db.GetPositionsBySession(ctx, s.ID) - if err != nil { - continue - } - for _, p := range positions { - if p.ExitTime != nil && p.RealizedPnL != nil { - returns = append(returns, *p.RealizedPnL) - } + for _, p := range positions { + if p.RealizedPnL != nil { + returns = append(returns, *p.RealizedPnL) } } return returns, nil diff --git a/internal/api/trades.go b/internal/api/trades.go index 4fbf93c..3a98d15 100644 --- a/internal/api/trades.go +++ b/internal/api/trades.go @@ -40,7 +40,8 @@ func (h *TradesHandler) ListTrades(c *gin.Context) { } } - trades, err := h.db.ListAllTrades(c.Request.Context(), limit, offset) + ctx := c.Request.Context() + trades, err := h.db.ListAllTrades(ctx, limit, offset) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to fetch trades"}) return @@ -48,9 +49,17 @@ func (h *TradesHandler) ListTrades(c *gin.Context) { if trades == nil { trades = []*db.Trade{} } + + total, err := h.db.CountAllTrades(ctx) + if err != nil { + // Non-fatal: return the page without total rather than failing the request. + total = -1 + } + c.JSON(http.StatusOK, gin.H{ "trades": trades, "count": len(trades), + "total": total, "limit": limit, "offset": offset, }) diff --git a/internal/db/positions.go b/internal/db/positions.go index 5376abb..7a7bf59 100644 --- a/internal/db/positions.go +++ b/internal/db/positions.go @@ -353,6 +353,29 @@ func (db *DB) GetAllOpenPositions(ctx context.Context) ([]*Position, error) { return scanPositions(rows) } +// GetAllClosedPositions returns all positions that have been closed (exit_time IS NOT NULL) +// across all sessions. Use this instead of iterating sessions + GetPositionsBySession to avoid N+1. +func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { + query := ` + SELECT + id, session_id, symbol, exchange, side, entry_price, exit_price, + quantity, entry_time, exit_time, stop_loss, take_profit, + realized_pnl, unrealized_pnl, fees, entry_reason, exit_reason, + metadata, created_at, updated_at + FROM positions + WHERE exit_time IS NOT NULL AND realized_pnl IS NOT NULL + ORDER BY exit_time DESC + ` + + rows, err := db.pool.Query(ctx, query) + if err != nil { + return nil, fmt.Errorf("failed to query closed positions: %w", err) + } + defer rows.Close() + + return scanPositions(rows) +} + // GetPositionsBySession retrieves all positions (including closed) for a session func (db *DB) GetPositionsBySession(ctx context.Context, sessionID uuid.UUID) ([]*Position, error) { query := ` diff --git a/internal/db/trades.go b/internal/db/trades.go index 53d3d17..b0c7407 100644 --- a/internal/db/trades.go +++ b/internal/db/trades.go @@ -2,6 +2,7 @@ package db import ( "context" + "fmt" ) // ListAllTrades returns recent trade fills across all orders, newest first. @@ -34,3 +35,12 @@ func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, e } return trades, rows.Err() } + +// CountAllTrades returns the total number of trade fill records in the database. +func (db *DB) CountAllTrades(ctx context.Context) (int, error) { + var count int + if err := db.pool.QueryRow(ctx, "SELECT COUNT(*) FROM trades").Scan(&count); err != nil { + return 0, fmt.Errorf("failed to count trades: %w", err) + } + return count, nil +} From 39ad6e9b4685d2a15f45bedf727a10da5bbfc898 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 23:01:05 +0530 Subject: [PATCH 09/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20second?= =?UTF-8?q?=20review=20=E2=80=94=20transactions,=20SQL=20aggregation,=20co?= =?UTF-8?q?nfig,=20auth,=20frontend?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - handlers_trading.go: wrap fill pipeline (InsertOrder→InsertTrade→CreatePosition) in pgx transaction for all-or-nothing semantics; rollback on any step failure - handlers_trading.go: return HTTP 422 for opposite-side trades instead of corrupting position data via averaging; prevents silent data corruption until close logic is impl - handlers_trading.go: move MockExchange to APIServer struct (shared instance, not per-req) - handlers_trading.go: set paper session Symbol to "PAPER" sentinel (multi-asset sessions) - handlers_trading.go: TOCTOU retry now guards on pgconn error code 23505 only - internal/api/risk.go: replace Go-side exposure aggregation with GetExposureBySymbol SQL GROUP BY; add cost-basis comment; move circuit breaker thresholds to RiskConfig; add VaR error logging; normalize PnL returns to fractional for VaR calculation - internal/api/performance.go: replace Go-side pair aggregation with GetPairPerformance SQL GROUP BY; remove aggregatePairPerformance method - internal/api/trades.go: change total sentinel from -1 to 0 with zerolog warning - internal/api/risk.go + trades.go + performance.go: add authMiddleware param to RegisterRoutes; applied on all routes; routes.go passes AuthMiddleware at call sites - internal/db/positions.go: add GetPairPerformance and GetExposureBySymbol SQL methods; add LIMIT 1000 ORDER BY exit_time DESC to GetAllClosedPositions - internal/db/trades.go: wrap scan/query errors with fmt.Errorf context; use O(1) pg_class.reltuples approximate count instead of SELECT COUNT(*) - internal/config/config.go: add MaxDailyLossDollars, MaxDrawdownPct, MaxTradeCount to RiskConfig with defaults; configs/config.yaml updated to match - cmd/api/main.go: initialize shared exchange.MockExchange on APIServer struct Frontend: - app/risk/page.tsx: destructure isError/isLoading from all three hooks; render error banner with retry button and loading indicator when API unavailable - app/risk/page.tsx: VaR values use formatCurrency (dollar amounts, not percentages); threshold updated to dollar-based comparison - app/risk/page.tsx: riskScore defaults to null → "Unknown"/"—" when no data - app/risk/page.tsx: remove hardcoded side:'long' from exposure mapping - hooks/useTradeData.ts: revert equity curve to getMockEquityPoints() with TODO comment - hooks/useTradeData.ts: add comment that mock mode ignores pagination params Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 271 ++++++++++++++++++++-------- cmd/api/main.go | 3 + cmd/api/routes.go | 8 +- configs/config.yaml | 6 + internal/api/performance.go | 64 +++---- internal/api/risk.go | 85 ++++++--- internal/api/trades.go | 16 +- internal/config/config.go | 14 ++ internal/db/positions.go | 81 ++++++++- internal/db/trades.go | 21 ++- web/dashboard/app/risk/page.tsx | 57 ++++-- web/dashboard/hooks/useTradeData.ts | 53 +++--- 12 files changed, 476 insertions(+), 203 deletions(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index fa6c61f..b373de8 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -2,6 +2,7 @@ package main import ( "errors" + "fmt" "net/http" "strings" "time" @@ -9,12 +10,14 @@ import ( "github.com/gin-gonic/gin" "github.com/google/uuid" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/rs/zerolog/log" "github.com/ajitpratap0/cryptofunk/internal/db" - "github.com/ajitpratap0/cryptofunk/internal/exchange" ) +// ptrStr and ptrF64 are local pointer helpers used only within this file. +// They are not duplicated elsewhere in the codebase (verified via grep). func ptrStr(s string) *string { return &s } func ptrF64(f float64) *float64 { return &f } @@ -426,7 +429,7 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { newSession := &db.TradingSession{ ID: uuid.New(), Mode: db.TradingModePaper, - Symbol: req.Symbol, + Symbol: "PAPER", // Symbol is intentionally generic; paper sessions are multi-asset Exchange: "paper", InitialCapital: 100_000.0, StartedAt: time.Now(), @@ -434,22 +437,34 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { UpdatedAt: time.Now(), } if err := s.db.CreateSession(ctx, newSession); err != nil { - // Another concurrent request may have created a paper session between the - // ListActiveSessions call above and this insert (TOCTOU). Try to look it up. - log.Warn().Err(err).Msg("Failed to create paper session; retrying lookup for concurrent session") - sessions2, err2 := s.db.ListActiveSessions(ctx) - if err2 == nil { - for i := range sessions2 { - if sessions2[i].Mode == db.TradingModePaper { - id := sessions2[i].ID - sessionID = &id - break + // Only retry on unique constraint violation (PG error code 23505). + // A concurrent request may have inserted the same paper session between the + // ListActiveSessions call above and this insert (TOCTOU race). In that case + // we look up and reuse the existing session. + // All other errors (e.g. connection timeout) are returned immediately. + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == "23505" { + log.Warn().Err(err).Msg("Unique constraint on paper session; retrying lookup for concurrent session") + sessions2, err2 := s.db.ListActiveSessions(ctx) + if err2 == nil { + for i := range sessions2 { + if sessions2[i].Mode == db.TradingModePaper { + id := sessions2[i].ID + sessionID = &id + break + } } } - } - if sessionID == nil { - log.Error().Err(err).Msg("Failed to create or find paper session") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create trading session"}) + if sessionID == nil { + log.Error().Err(err).Msg("Failed to find paper session after unique constraint violation") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create trading session"}) + return + } + } else { + log.Error().Err(err).Msg("Failed to create paper session") + c.JSON(http.StatusInternalServerError, gin.H{ + "error": fmt.Sprintf("failed to create paper session: %v", err), + }) return } } else { @@ -460,8 +475,7 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { // 2. Determine execution price refPrice := req.Price if !isLimit && refPrice <= 0 { - mockEx := exchange.NewMockExchange(s.db) - refPrice = mockEx.GetMarketPrice(req.Symbol) + refPrice = s.exchange.GetMarketPrice(req.Symbol) if refPrice <= 0 { c.JSON(http.StatusBadRequest, gin.H{ "error": "no market price configured for symbol; provide a price field", @@ -510,91 +524,194 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { // 4. Immediate fill for market orders if order.Type == db.OrderTypeMarket { execQuoteQty := execPrice * req.Quantity - commission := execQuoteQty * 0.001 - - if err := s.db.UpdateOrderStatus(ctx, order.ID, db.OrderStatusFilled, - req.Quantity, execQuoteQty, &now, nil, nil); err != nil { - log.Warn().Err(err).Str("order_id", order.ID.String()).Msg("Failed to mark paper order filled") - } else { - order.Status = db.OrderStatusFilled - order.ExecutedQuantity = req.Quantity - order.ExecutedQuoteQuantity = execQuoteQty - order.FilledAt = &now - order.UpdatedAt = now + // Use the configured paper trading commission rate (taker fee from trading config). + // Falls back to 0.001 (0.1%) if not configured, matching Binance standard tier. + commissionRate := s.config.Trading.CommissionRate + if commissionRate <= 0 { + commissionRate = 0.001 } + commission := execQuoteQty * commissionRate - // Write fill record - commissionAsset := "USDT" - trade := &db.Trade{ - ID: uuid.New(), - OrderID: order.ID, - Symbol: req.Symbol, - Exchange: "paper", - Side: orderSide, - Price: execPrice, - Quantity: req.Quantity, - QuoteQuantity: execQuoteQty, - Commission: commission, - CommissionAsset: &commissionAsset, - ExecutedAt: now, - IsMaker: false, - CreatedAt: now, - } - if err := s.db.InsertTrade(ctx, trade); err != nil { - log.Warn().Err(err).Msg("Failed to insert paper trade fill row") + posSide := db.PositionSideLong + if orderSide == db.OrderSideSell { + posSide = db.PositionSideShort } - // Create or average into existing position + // Check for an opposite-side position before opening a transaction so we + // can reject early without acquiring DB resources unnecessarily. existingPos, posErr := s.db.GetPositionBySymbolAndSession(ctx, req.Symbol, *sessionID) if posErr != nil && !errors.Is(posErr, pgx.ErrNoRows) { log.Warn().Err(posErr).Msg("Error looking up existing position") } - - posSide := db.PositionSideLong - if orderSide == db.OrderSideSell { - posSide = db.PositionSideShort - } - if existingPos != nil && existingPos.Side != posSide { // Opposite-side trade on an existing open position. Proper close/reduce // logic (netting, realized PnL calculation) is not yet implemented. - // For now we average into the opposite direction, which is incorrect for - // a long→short flip. This is a known limitation. - // TODO: implement position close/reduce logic. + // Reject the trade rather than silently corrupting position data. log.Warn(). Str("symbol", req.Symbol). Str("existing_side", string(existingPos.Side)). Str("order_side", string(posSide)). Msg("Opposite-side trade on existing position; position close logic not yet implemented") + c.JSON(http.StatusUnprocessableEntity, gin.H{ + "error": "position close/reduce not yet implemented; opposite-side trade rejected", + }) + return } - if existingPos == nil { - pos := &db.Position{ - ID: uuid.New(), - SessionID: sessionID, - Symbol: req.Symbol, - Exchange: "paper", - Side: posSide, - EntryPrice: execPrice, - Quantity: req.Quantity, - EntryTime: now, - Fees: commission, - EntryReason: ptrStr("paper_trade_api"), - UnrealizedPnL: ptrF64(0), - CreatedAt: now, - UpdatedAt: now, + // Wrap the fill writes (UpdateOrderStatus → InsertTrade → CreatePosition/UpdatePositionAveraging) + // in a single DB transaction so a mid-flight failure does not leave orphaned rows. + // AggregateSessionStats is intentionally kept outside the transaction: it is a + // read-then-aggregate UPDATE that can be safely retried and does not create new rows. + tx, txErr := s.db.Pool().BeginTx(ctx, pgx.TxOptions{}) + if txErr != nil { + log.Error().Err(txErr).Msg("Failed to begin paper trade transaction") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to begin paper trade transaction"}) + return + } + // rollbackTx is a no-op after a successful Commit. + rollbackTx := func() { + if rbErr := tx.Rollback(ctx); rbErr != nil && !errors.Is(rbErr, pgx.ErrTxClosed) { + log.Warn().Err(rbErr).Msg("Failed to rollback paper trade transaction") } - if err := s.db.CreatePosition(ctx, pos); err != nil { - log.Warn().Err(err).Msg("Failed to create position for paper trade") + } + + // UpdateOrderStatus inside transaction + _, txErr = tx.Exec(ctx, ` + UPDATE orders + SET status = $1, + executed_quantity = $2, + executed_quote_quantity = $3, + filled_at = $4, + canceled_at = $5, + error_message = $6, + updated_at = NOW() + WHERE id = $7`, + db.OrderStatusFilled, + req.Quantity, + execQuoteQty, + &now, + nil, + nil, + order.ID, + ) + if txErr != nil { + rollbackTx() + log.Error().Err(txErr).Str("order_id", order.ID.String()).Msg("Failed to mark paper order filled in transaction") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to fill paper trade order"}) + return + } + order.Status = db.OrderStatusFilled + order.ExecutedQuantity = req.Quantity + order.ExecutedQuoteQuantity = execQuoteQty + order.FilledAt = &now + order.UpdatedAt = now + + // InsertTrade inside transaction + commissionAsset := "USDT" + tradeID := uuid.New() + _, txErr = tx.Exec(ctx, ` + INSERT INTO trades ( + id, order_id, exchange_trade_id, symbol, exchange, side, + price, quantity, quote_quantity, commission, commission_asset, + executed_at, is_maker, metadata, created_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 + )`, + tradeID, + order.ID, + nil, + req.Symbol, + "paper", + orderSide, + execPrice, + req.Quantity, + execQuoteQty, + commission, + &commissionAsset, + now, + false, + nil, + now, + ) + if txErr != nil { + rollbackTx() + log.Error().Err(txErr).Msg("Failed to insert paper trade fill row in transaction") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to record paper trade fill"}) + return + } + + // Create or average into existing position inside transaction + if existingPos == nil { + posID := uuid.New() + entryReason := "paper_trade_api" + unrealizedPnL := 0.0 + _, txErr = tx.Exec(ctx, ` + INSERT INTO positions ( + id, session_id, symbol, exchange, side, entry_price, quantity, + entry_time, stop_loss, take_profit, entry_reason, metadata, created_at, updated_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14 + )`, + posID, + sessionID, + req.Symbol, + "paper", + posSide, + execPrice, + req.Quantity, + now, + nil, + nil, + &entryReason, + nil, + now, + now, + ) + if txErr != nil { + rollbackTx() + log.Error().Err(txErr).Msg("Failed to create position for paper trade in transaction") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create position for paper trade"}) + return } + // Persist unrealized_pnl separately as it is not in the INSERT above + _, _ = tx.Exec(ctx, + `UPDATE positions SET unrealized_pnl = $1 WHERE id = $2`, + unrealizedPnL, posID, + ) } else { totalQty := existingPos.Quantity + req.Quantity weightedAvg := (existingPos.Quantity*existingPos.EntryPrice + req.Quantity*execPrice) / totalQty - if err := s.db.UpdatePositionAveraging(ctx, existingPos.ID, weightedAvg, totalQty, commission); err != nil { - log.Warn().Err(err).Msg("Failed to update position for paper trade") + _, txErr = tx.Exec(ctx, ` + UPDATE positions + SET + entry_price = $2, + quantity = $3, + fees = fees + $4, + updated_at = $5 + WHERE id = $1 AND exit_time IS NULL`, + existingPos.ID, + weightedAvg, + totalQty, + commission, + now, + ) + if txErr != nil { + rollbackTx() + log.Error().Err(txErr).Msg("Failed to update position for paper trade in transaction") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to update position for paper trade"}) + return } } + if commitErr := tx.Commit(ctx); commitErr != nil { + rollbackTx() + log.Error().Err(commitErr).Msg("Failed to commit paper trade transaction") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to commit paper trade"}) + return + } + + // AggregateSessionStats is outside the transaction: it is a safe read-aggregate + // UPDATE that can be retried without risk of partial data corruption. if err := s.db.AggregateSessionStats(ctx, *sessionID); err != nil { log.Warn().Err(err).Msg("Failed to aggregate session stats after paper trade") } diff --git a/cmd/api/main.go b/cmd/api/main.go index 1eddbe0..bb26930 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -18,6 +18,7 @@ import ( "github.com/ajitpratap0/cryptofunk/internal/api" "github.com/ajitpratap0/cryptofunk/internal/config" "github.com/ajitpratap0/cryptofunk/internal/db" + "github.com/ajitpratap0/cryptofunk/internal/exchange" "github.com/ajitpratap0/cryptofunk/internal/safety" ) @@ -43,6 +44,7 @@ type APIServer struct { orderExecSession *mcp.ClientSession // MCP session for order-executor calls mcpClient *mcp.Client // MCP client for creating/reconnecting sessions activeSessionID *uuid.UUID // Currently active trading session ID (guarded by sessionMu) + exchange exchange.Exchange // Shared mock exchange instance for paper trading } // HTTP client for orchestrator communication with timeout and connection pooling @@ -122,6 +124,7 @@ func main() { ctx: ctx, safetyGuard: safetyGuard, orderExecutorURL: getOrderExecutorURL(), + exchange: exchange.NewMockExchange(database), } // Initialize MCP client for order-executor (session connects lazily on first order) diff --git a/cmd/api/routes.go b/cmd/api/routes.go index 8f1736f..9eb3479 100644 --- a/cmd/api/routes.go +++ b/cmd/api/routes.go @@ -238,7 +238,7 @@ func (s *APIServer) setupRoutes() { // Trades routes — fill records from the trades table tradesHandler := api.NewTradesHandler(s.db) - tradesHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware()) + tradesHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware(), api.AuthMiddleware(s.apiKeyStore, authConfig)) // TB-006: API Key Management routes // These endpoints allow users to manage their API keys (create, rotate, revoke) @@ -265,12 +265,12 @@ func (s *APIServer) setupRoutes() { polymarketHandler.RegisterRoutesWithRateLimiter(v1, s.rateLimiter.ReadMiddleware(), s.rateLimiter.OrderMiddleware()) // Risk metrics routes - riskHandler := api.NewRiskHandler(s.db) - riskHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware()) + riskHandler := api.NewRiskHandler(s.db, &s.config.Risk) + riskHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware(), api.AuthMiddleware(s.apiKeyStore, authConfig)) // Performance routes perfHandler := api.NewPerformanceHandler(s.db) - perfHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware()) + perfHandler.RegisterRoutes(v1, s.rateLimiter.ReadMiddleware(), api.AuthMiddleware(s.apiKeyStore, authConfig)) // Decision analytics and outcome resolution routes decisionAnalyticsHandler := api.NewDecisionAnalyticsHandler(s.db) diff --git a/configs/config.yaml b/configs/config.yaml index 9b00412..05d382a 100644 --- a/configs/config.yaml +++ b/configs/config.yaml @@ -153,6 +153,7 @@ trading: initial_capital: 10000.0 max_positions: 3 default_quantity: 0.01 + commission_rate: 0.001 # 0.1% taker fee applied to paper trade fills (Binance standard tier) risk: max_position_size: 0.1 # 10% of portfolio @@ -163,6 +164,11 @@ risk: llm_approval_required: true # Require LLM approval for trades min_confidence: 0.7 # Minimum confidence for signals + # Dashboard circuit breaker thresholds (absolute units, used by /risk/circuit-breakers) + max_daily_loss_dollars: 5000.0 # Trigger when cumulative session loss exceeds $5000 + max_drawdown_pct: 10.0 # Trigger when max session drawdown exceeds 10% + max_trade_count: 100 # Trigger when total trades across active sessions exceeds 100 + # Safety Guard Configuration # Live trading protection mechanisms to prevent excessive losses # These guards are enforced at the order execution level diff --git a/internal/api/performance.go b/internal/api/performance.go index 605608a..a971c60 100644 --- a/internal/api/performance.go +++ b/internal/api/performance.go @@ -1,7 +1,6 @@ package api import ( - "context" "math" "net/http" @@ -21,10 +20,15 @@ func NewPerformanceHandler(database *db.DB) *PerformanceHandler { } // RegisterRoutes mounts the /performance sub-group under the provided router group. -func (h *PerformanceHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc) { +// If authMiddleware is non-nil it is applied to all routes alongside readMiddleware. +func (h *PerformanceHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc, authMiddleware gin.HandlerFunc) { g := rg.Group("/performance") - g.Use(readMiddleware) - g.GET("/pairs", h.GetPairPerformance) + if authMiddleware != nil { + g.GET("/pairs", readMiddleware, authMiddleware, h.GetPairPerformance) + } else { + g.Use(readMiddleware) + g.GET("/pairs", h.GetPairPerformance) + } } // GetPairPerformance returns realized PnL aggregated by trading pair across all active sessions. @@ -32,51 +36,25 @@ func (h *PerformanceHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware func (h *PerformanceHandler) GetPairPerformance(c *gin.Context) { ctx := c.Request.Context() - pairs, err := h.aggregatePairPerformance(ctx) + rows, err := h.db.GetPairPerformance(ctx) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to aggregate pair performance"}) return } - c.JSON(http.StatusOK, gin.H{"pairs": pairs, "count": len(pairs)}) -} - -type pairPerf struct { - Symbol string `json:"symbol"` - RealizedPnL float64 `json:"realized_pnl"` - TradeCount int `json:"trade_count"` -} - -func (h *PerformanceHandler) aggregatePairPerformance(ctx context.Context) ([]pairPerf, error) { - // Fetch all closed positions in one query to avoid N+1 per session. - positions, err := h.db.GetAllClosedPositions(ctx) - if err != nil { - return nil, err - } - - type agg struct { - pnl float64 - count int - } - bySymbol := make(map[string]*agg) - - for _, p := range positions { - if p.RealizedPnL != nil { - if _, ok := bySymbol[p.Symbol]; !ok { - bySymbol[p.Symbol] = &agg{} - } - bySymbol[p.Symbol].pnl += *p.RealizedPnL - bySymbol[p.Symbol].count++ - } + type pairPerf struct { + Symbol string `json:"symbol"` + RealizedPnL float64 `json:"realized_pnl"` + TradeCount int `json:"trade_count"` } - - result := make([]pairPerf, 0, len(bySymbol)) - for sym, a := range bySymbol { - result = append(result, pairPerf{ - Symbol: sym, - RealizedPnL: math.Round(a.pnl*100) / 100, - TradeCount: a.count, + pairs := make([]pairPerf, 0, len(rows)) + for _, r := range rows { + pairs = append(pairs, pairPerf{ + Symbol: r.Symbol, + RealizedPnL: math.Round(r.RealizedPnL*100) / 100, + TradeCount: r.TradeCount, }) } - return result, nil + + c.JSON(http.StatusOK, gin.H{"pairs": pairs, "count": len(pairs)}) } diff --git a/internal/api/risk.go b/internal/api/risk.go index c66b734..5f179a7 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -6,7 +6,9 @@ import ( "net/http" "github.com/gin-gonic/gin" + "github.com/rs/zerolog/log" + "github.com/ajitpratap0/cryptofunk/internal/config" "github.com/ajitpratap0/cryptofunk/internal/db" "github.com/ajitpratap0/cryptofunk/internal/risk" ) @@ -15,23 +17,37 @@ import ( type RiskHandler struct { db *db.DB riskService *risk.Service + cfg *config.RiskConfig } -// NewRiskHandler creates a new RiskHandler backed by the given database. -func NewRiskHandler(database *db.DB) *RiskHandler { +// NewRiskHandler creates a new RiskHandler backed by the given database and risk config. +// If cfg is nil a zero-value RiskConfig is used, which causes setDefaults values to be +// applied at load time and therefore cfg should always be non-nil in production. +func NewRiskHandler(database *db.DB, cfg *config.RiskConfig) *RiskHandler { + if cfg == nil { + cfg = &config.RiskConfig{} + } return &RiskHandler{ db: database, riskService: risk.NewService(), + cfg: cfg, } } // RegisterRoutes mounts the /risk sub-group under the provided router group. -func (h *RiskHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc) { +// If authMiddleware is non-nil it is applied to all routes alongside readMiddleware. +func (h *RiskHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc, authMiddleware gin.HandlerFunc) { r := rg.Group("/risk") - r.Use(readMiddleware) - r.GET("/metrics", h.GetMetrics) - r.GET("/circuit-breakers", h.GetCircuitBreakers) - r.GET("/exposure", h.GetExposure) + if authMiddleware != nil { + r.GET("/metrics", readMiddleware, authMiddleware, h.GetMetrics) + r.GET("/circuit-breakers", readMiddleware, authMiddleware, h.GetCircuitBreakers) + r.GET("/exposure", readMiddleware, authMiddleware, h.GetExposure) + } else { + r.Use(readMiddleware) + r.GET("/metrics", h.GetMetrics) + r.GET("/circuit-breakers", h.GetCircuitBreakers) + r.GET("/exposure", h.GetExposure) + } } // GetMetrics returns VaR, CVaR, open position count, and total exposure. @@ -46,6 +62,9 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { } openCount := len(openPositions) + // NOTE: exposure is calculated at cost-basis (entry_price), not mark-to-market. + // Current market price is not stored on the position; a live price lookup + // would be needed for accurate mark-to-market exposure. var totalExposure float64 for _, p := range openPositions { totalExposure += p.Quantity * p.EntryPrice @@ -77,7 +96,9 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { "returns": returnsIface, "confidence_level": 0.95, }) - if err == nil { + if err != nil { + log.Debug().Err(err).Msg("VaR calculation failed (95%)") + } else { if varResult, ok := res95.(*risk.VaRResult); ok { response["var_95"] = varResult.VaR } @@ -87,7 +108,9 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { "returns": returnsIface, "confidence_level": 0.99, }) - if err == nil { + if err != nil { + log.Debug().Err(err).Msg("VaR calculation failed (99%)") + } else { if varResult, ok := res99.(*risk.VaRResult); ok { response["var_99"] = varResult.VaR response["expected_shortfall"] = varResult.CVaR @@ -119,12 +142,12 @@ func (h *RiskHandler) GetCircuitBreakers(c *gin.Context) { totalTrades += s.TotalTrades } - // Max Daily Loss: triggered when cumulative losses exceed $5000 + // Max Daily Loss: triggered when cumulative losses exceed the configured threshold (dollars). lossAmount := math.Abs(math.Min(totalPnL, 0)) breakers := []gin.H{ - buildBreaker("Max Daily Loss", lossAmount, 5000), - buildBreaker("Max Drawdown %", maxDrawdown*100, 10), - buildBreaker("Total Trade Count", float64(totalTrades), 100), + buildBreaker("Max Daily Loss", lossAmount, h.cfg.MaxDailyLossDollars), + buildBreaker("Max Drawdown %", maxDrawdown*100, h.cfg.MaxDrawdownPct), + buildBreaker("Total Trade Count", float64(totalTrades), float64(h.cfg.MaxTradeCount)), } c.JSON(http.StatusOK, gin.H{"circuit_breakers": breakers, "count": len(breakers)}) @@ -150,33 +173,33 @@ func buildBreaker(name string, current, threshold float64) gin.H { func (h *RiskHandler) GetExposure(c *gin.Context) { ctx := c.Request.Context() - openPositions, err := h.db.GetAllOpenPositions(ctx) + // NOTE: exposure is calculated at cost-basis (entry_price), not mark-to-market. + // Current market price is not stored on the position; a live price lookup + // would be needed for accurate mark-to-market exposure. + rows, err := h.db.GetExposureBySymbol(ctx) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to query open positions"}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to query exposure by symbol"}) return } - exposureBySymbol := make(map[string]float64) - for _, p := range openPositions { - exposureBySymbol[p.Symbol] += p.Quantity * p.EntryPrice - } - - type symbolExposure struct { + type symbolExposureJSON struct { Symbol string `json:"symbol"` Exposure float64 `json:"exposure"` } - result := make([]symbolExposure, 0, len(exposureBySymbol)) - for sym, exp := range exposureBySymbol { - result = append(result, symbolExposure{ - Symbol: sym, - Exposure: math.Round(exp*100) / 100, + result := make([]symbolExposureJSON, 0, len(rows)) + for _, r := range rows { + result = append(result, symbolExposureJSON{ + Symbol: r.Symbol, + Exposure: math.Round(r.Exposure*100) / 100, }) } c.JSON(http.StatusOK, gin.H{"exposure": result, "count": len(result)}) } -// collectClosedReturns gathers RealizedPnL from all closed positions in a single query. +// collectClosedReturns gathers fractional returns from all closed positions. +// Each return is RealizedPnL / (EntryPrice * Quantity) so values are dimensionless +// fractions (e.g. 0.023 = 2.3%) suitable for VaR calculations. func (h *RiskHandler) collectClosedReturns(ctx context.Context) ([]float64, error) { positions, err := h.db.GetAllClosedPositions(ctx) if err != nil { @@ -185,8 +208,14 @@ func (h *RiskHandler) collectClosedReturns(ctx context.Context) ([]float64, erro var returns []float64 for _, p := range positions { + // RealizedPnL is non-nil here: GetAllClosedPositions filters realized_pnl IS NOT NULL. + // This guard is retained as defense-in-depth against future query changes. if p.RealizedPnL != nil { - returns = append(returns, *p.RealizedPnL) + notional := p.EntryPrice * p.Quantity + if notional == 0 { + continue + } + returns = append(returns, *p.RealizedPnL/notional) } } return returns, nil diff --git a/internal/api/trades.go b/internal/api/trades.go index 3a98d15..11bd2fb 100644 --- a/internal/api/trades.go +++ b/internal/api/trades.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/gin-gonic/gin" + "github.com/rs/zerolog/log" "github.com/ajitpratap0/cryptofunk/internal/db" ) @@ -18,10 +19,16 @@ func NewTradesHandler(database *db.DB) *TradesHandler { return &TradesHandler{db: database} } -func (h *TradesHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc) { +// RegisterRoutes mounts the /trades sub-group under the provided router group. +// If authMiddleware is non-nil it is applied to all routes alongside readMiddleware. +func (h *TradesHandler) RegisterRoutes(rg *gin.RouterGroup, readMiddleware gin.HandlerFunc, authMiddleware gin.HandlerFunc) { trades := rg.Group("/trades") - trades.Use(readMiddleware) - trades.GET("", h.ListTrades) + if authMiddleware != nil { + trades.GET("", readMiddleware, authMiddleware, h.ListTrades) + } else { + trades.Use(readMiddleware) + trades.GET("", h.ListTrades) + } } // ListTrades returns recent trade fills, newest first. @@ -53,7 +60,8 @@ func (h *TradesHandler) ListTrades(c *gin.Context) { total, err := h.db.CountAllTrades(ctx) if err != nil { // Non-fatal: return the page without total rather than failing the request. - total = -1 + log.Warn().Err(err).Msg("failed to count trades, total will be 0") + total = 0 } c.JSON(http.StatusOK, gin.H{ diff --git a/internal/config/config.go b/internal/config/config.go index 26773b8..f0109b1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -136,6 +136,7 @@ type TradingConfig struct { InitialCapital float64 `mapstructure:"initial_capital"` // 10000.0 MaxPositions int `mapstructure:"max_positions"` // 3 DefaultQuantity float64 `mapstructure:"default_quantity"` // 0.01 + CommissionRate float64 `mapstructure:"commission_rate"` // 0.001 (0.1%) — paper trade taker fee } // RiskConfig contains risk management settings @@ -149,6 +150,13 @@ type RiskConfig struct { MinConfidence float64 `mapstructure:"min_confidence"` // 0.7 CircuitBreaker CircuitBreakerConfig `mapstructure:"circuit_breaker"` // Circuit breaker thresholds SafetyGuard SafetyGuardConfig `mapstructure:"safety_guard"` // Live trading safety guards + + // Dashboard circuit breaker thresholds — used by the /risk/circuit-breakers endpoint. + // These are expressed in absolute units: dollars for MaxDailyLossDollars, + // percentage points for MaxDrawdownPct, and a count for MaxTradeCount. + MaxDailyLossDollars float64 `mapstructure:"max_daily_loss_dollars"` // 5000.0 (USD) + MaxDrawdownPct float64 `mapstructure:"max_drawdown_pct"` // 10.0 (percent) + MaxTradeCount int `mapstructure:"max_trade_count"` // 100 } // SafetyGuardConfig contains safety guard settings for live trading protection @@ -434,6 +442,7 @@ func setDefaults(v *viper.Viper) { v.SetDefault("trading.initial_capital", 10000.0) v.SetDefault("trading.max_positions", 3) v.SetDefault("trading.default_quantity", 0.01) + v.SetDefault("trading.commission_rate", 0.001) // 0.1% taker fee (Binance standard tier) // Risk defaults v.SetDefault("risk.max_position_size", 0.1) @@ -444,6 +453,11 @@ func setDefaults(v *viper.Viper) { v.SetDefault("risk.llm_approval_required", true) v.SetDefault("risk.min_confidence", 0.7) + // Dashboard circuit breaker threshold defaults (absolute units) + v.SetDefault("risk.max_daily_loss_dollars", 5000.0) + v.SetDefault("risk.max_drawdown_pct", 10.0) + v.SetDefault("risk.max_trade_count", 100) + // Circuit Breaker defaults - aligned with configs/circuit_breakers.yaml // Exchange v.SetDefault("risk.circuit_breaker.exchange.min_requests", 5) diff --git a/internal/db/positions.go b/internal/db/positions.go index 7a7bf59..804cf02 100644 --- a/internal/db/positions.go +++ b/internal/db/positions.go @@ -353,8 +353,9 @@ func (db *DB) GetAllOpenPositions(ctx context.Context) ([]*Position, error) { return scanPositions(rows) } -// GetAllClosedPositions returns all positions that have been closed (exit_time IS NOT NULL) -// across all sessions. Use this instead of iterating sessions + GetPositionsBySession to avoid N+1. +// GetAllClosedPositions returns the most recent 1000 closed positions across all sessions, +// ordered by exit_time DESC. VaR and performance calculations only need recent history, +// and a full table scan becomes expensive at scale. func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { query := ` SELECT @@ -365,6 +366,7 @@ func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { FROM positions WHERE exit_time IS NOT NULL AND realized_pnl IS NOT NULL ORDER BY exit_time DESC + LIMIT 1000 ` rows, err := db.pool.Query(ctx, query) @@ -657,6 +659,81 @@ func (db *DB) PartialClosePosition(ctx context.Context, id uuid.UUID, closeQuant return closedPosition, nil } +// PairPerformance holds aggregated realized PnL and trade count for a single trading pair. +type PairPerformance struct { + Symbol string `db:"symbol"` + RealizedPnL float64 `db:"realized_pnl"` + TradeCount int `db:"trade_count"` +} + +// GetPairPerformance returns realized PnL and trade count grouped by symbol using SQL GROUP BY, +// covering all closed positions where realized_pnl is not NULL. +func (db *DB) GetPairPerformance(ctx context.Context) ([]PairPerformance, error) { + query := ` + SELECT symbol, COALESCE(SUM(realized_pnl), 0) AS realized_pnl, COUNT(*) AS trade_count + FROM positions + WHERE exit_time IS NOT NULL AND realized_pnl IS NOT NULL + GROUP BY symbol + ORDER BY realized_pnl DESC + ` + + rows, err := db.pool.Query(ctx, query) + if err != nil { + return nil, fmt.Errorf("failed to query pair performance: %w", err) + } + defer rows.Close() + + var results []PairPerformance + for rows.Next() { + var p PairPerformance + if err := rows.Scan(&p.Symbol, &p.RealizedPnL, &p.TradeCount); err != nil { + return nil, fmt.Errorf("failed to scan pair performance row: %w", err) + } + results = append(results, p) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating pair performance rows: %w", err) + } + return results, nil +} + +// SymbolExposure holds the cost-basis exposure for a single symbol across all open positions. +type SymbolExposure struct { + Symbol string `db:"symbol"` + Exposure float64 `db:"exposure"` +} + +// GetExposureBySymbol returns the total open-position exposure (quantity * entry_price) grouped by +// symbol using SQL GROUP BY. Exposure is calculated at cost-basis, not mark-to-market. +func (db *DB) GetExposureBySymbol(ctx context.Context) ([]SymbolExposure, error) { + query := ` + SELECT symbol, SUM(quantity * entry_price) AS exposure + FROM positions + WHERE exit_time IS NULL + GROUP BY symbol + ORDER BY exposure DESC + ` + + rows, err := db.pool.Query(ctx, query) + if err != nil { + return nil, fmt.Errorf("failed to query exposure by symbol: %w", err) + } + defer rows.Close() + + var results []SymbolExposure + for rows.Next() { + var s SymbolExposure + if err := rows.Scan(&s.Symbol, &s.Exposure); err != nil { + return nil, fmt.Errorf("failed to scan symbol exposure row: %w", err) + } + results = append(results, s) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating symbol exposure rows: %w", err) + } + return results, nil +} + // ConvertPositionSide converts a string to PositionSide func ConvertPositionSide(side string) PositionSide { switch side { diff --git a/internal/db/trades.go b/internal/db/trades.go index b0c7407..bc39ca1 100644 --- a/internal/db/trades.go +++ b/internal/db/trades.go @@ -17,7 +17,7 @@ func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, e LIMIT $1 OFFSET $2 `, limit, offset) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to query trades: %w", err) } defer rows.Close() @@ -29,18 +29,27 @@ func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, e &t.Price, &t.Quantity, &t.QuoteQuantity, &t.Commission, &t.CommissionAsset, &t.ExecutedAt, &t.IsMaker, &t.Metadata, &t.CreatedAt, ); err != nil { - return nil, err + return nil, fmt.Errorf("failed to scan trade row: %w", err) } trades = append(trades, t) } return trades, rows.Err() } -// CountAllTrades returns the total number of trade fill records in the database. +// CountAllTrades returns an approximate count of trade fill records using pg_class statistics. +// This is O(1) instead of O(n) — avoids a full sequential COUNT(*) scan on every request. +// The estimate is sourced from pg_class.reltuples which is updated by ANALYZE/autovacuum. +// If the table has never been analyzed (reltuples = -1), the function returns 0. func (db *DB) CountAllTrades(ctx context.Context) (int, error) { - var count int - if err := db.pool.QueryRow(ctx, "SELECT COUNT(*) FROM trades").Scan(&count); err != nil { + var estimate int64 + if err := db.pool.QueryRow(ctx, + "SELECT reltuples::bigint AS estimate FROM pg_class WHERE relname = 'trades'", + ).Scan(&estimate); err != nil { return 0, fmt.Errorf("failed to count trades: %w", err) } - return count, nil + // reltuples is -1 for a freshly created table that has never been analyzed. + if estimate < 0 { + return 0, nil + } + return int(estimate), nil } diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index 5768373..dde0ea5 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -13,8 +13,7 @@ import { ExposurePie } from '@/components/charts/ExposurePie' import { DrawdownChart } from '@/components/charts/DrawdownChart' import { CircuitBreakerStatus } from '@/components/risk/CircuitBreakerStatus' import { StatCard } from '@/components/ui/StatCard' -import { cn, formatCurrency, formatPercentage } from '@/lib/utils' -import { RISK_THRESHOLDS } from '@/lib/constants' +import { cn, formatCurrency } from '@/lib/utils' import { useRiskMetrics, useCircuitBreakers, useRiskExposure } from '@/hooks/usePerformance' import type { CircuitBreaker } from '@/lib/types' @@ -45,9 +44,12 @@ const mockAlerts = [ export default function RiskPage() { const [timeRange, setTimeRange] = useState<'1w' | '1m' | '3m'>('1m') - const { data: metricsResponse, refetch: refetchMetrics } = useRiskMetrics() - const { data: breakersResponse, refetch: refetchBreakers } = useCircuitBreakers() - const { data: exposureResponse, refetch: refetchExposure } = useRiskExposure() + const { data: metricsResponse, isError: metricsError, isLoading: metricsLoading, refetch: refetchMetrics } = useRiskMetrics() + const { data: breakersResponse, isError: breakersError, isLoading: breakersLoading, refetch: refetchBreakers } = useCircuitBreakers() + const { data: exposureResponse, isError: exposureError, isLoading: exposureLoading, refetch: refetchExposure } = useRiskExposure() + + const isLoading = metricsLoading || breakersLoading || exposureLoading + const hasError = metricsError || breakersError || exposureError const handleRefresh = () => { refetchMetrics() @@ -80,6 +82,9 @@ export default function RiskPage() { }, [breakersResponse]) // Map API exposure data + // Issue #2 fix: API GetExposure does not return a `side` field — omit it + // entirely so ExposurePie uses its default asset-based coloring instead of + // incorrectly coloring every bar as a long (profit/green). const exposureData = useMemo(() => { const raw = exposureResponse?.data as | { exposure: Array<{ symbol: string; exposure: number }> } @@ -91,15 +96,16 @@ export default function RiskPage() { exposure: e.exposure, percentage: total > 0 ? Math.round((e.exposure / total) * 100) : 0, value: e.exposure, - side: 'long' as const, })) }, [exposureResponse]) const var95 = apiMetrics?.var_95 ?? null const var99 = apiMetrics?.var_99 ?? null - const riskScore = useMemo(() => { - if (!circuitBreakers.length) return 100 + // Issue #7 fix: return null when circuit breaker data is unavailable so the + // UI shows "Unknown / —" rather than a misleading "Low Risk" score of 100. + const riskScore = useMemo(() => { + if (!circuitBreakers.length) return null let score = 100 circuitBreakers.forEach(cb => { const usage = cb.threshold > 0 ? Math.abs(cb.currentValue) / Math.abs(cb.threshold) : 0 @@ -110,11 +116,31 @@ export default function RiskPage() { return Math.max(0, Math.min(100, score)) }, [circuitBreakers]) - const riskLevel = riskScore >= 80 ? 'Low' : riskScore >= 50 ? 'Medium' : 'High' - const riskColor = riskScore >= 80 ? 'text-profit' : riskScore >= 50 ? 'text-warning' : 'text-loss' + const riskLevel = riskScore == null ? 'Unknown' : riskScore >= 80 ? 'Low' : riskScore >= 50 ? 'Medium' : 'High' + const riskColor = riskScore == null ? 'text-muted-foreground' : riskScore >= 80 ? 'text-profit' : riskScore >= 50 ? 'text-warning' : 'text-loss' return (
+ {/* Error / Loading banner */} + {hasError && ( +
+ Risk data unavailable — API error + +
+ )} + {isLoading && !hasError && ( +
+ + Loading risk data… +
+ )} + {/* Header */}
@@ -136,17 +162,20 @@ export default function RiskPage() {
} valueClassName={riskColor} /> + {/* Issue #1 fix: var95/var99 are dollar PnL values, not fractional + percentages — use formatCurrency instead of formatPercentage. + Threshold comparison updated to dollar-based (>$500 = warning). */} } - valueClassName={var95 != null && var95 > RISK_THRESHOLDS.var.warning ? 'text-warning' : 'text-foreground'} + valueClassName={var95 != null && Math.abs(var95) > 500 ? 'text-warning' : 'text-foreground'} /> { if (USE_MOCK_DATA) { + // NOTE: mock mode returns all trades regardless of limit/offset params. + // Pagination is only enforced against the real API. return { success: true as const, data: getMockTrades(), @@ -96,11 +98,11 @@ export function usePositions() { } const response = await apiClient.getPositions() - + if (!response.success) { throw new Error(response.error || 'Failed to fetch positions') } - + // API returns {positions: [...], count: N} - extract the array const rawData: unknown = response.data if (isWrappedResponse(rawData)) { @@ -110,7 +112,7 @@ export function usePositions() { timestamp: response.timestamp, } } - + return response }, staleTime: REFRESH_INTERVALS.positions, @@ -141,11 +143,11 @@ export function useOrders() { } const response = await apiClient.getOrders() - + if (!response.success) { throw new Error(response.error || 'Failed to fetch orders') } - + // API returns {orders: [...], count: N} - extract the array const rawData: unknown = response.data if (isWrappedOrders(rawData)) { @@ -155,7 +157,7 @@ export function useOrders() { timestamp: response.timestamp, } } - + return response }, staleTime: REFRESH_INTERVALS.trades, @@ -177,17 +179,17 @@ export function useDashboard() { } const response = await apiClient.getDashboard() - + if (!response.success) { throw new Error(response.error || 'Failed to fetch dashboard') } - + // Transform API response shape to DashboardStats const raw: unknown = response.data if (isRawDashboardResponse(raw)) { const pnl = raw.pnl_summary || {} const pos = raw.position_summary || {} - + const transformed: DashboardStats = { totalPnl: pnl.total_pnl ?? 0, totalPnlPercent: pnl.return_percent ?? 0, @@ -199,14 +201,14 @@ export function useDashboard() { marginUsed: pos.total_exposure ?? 0, marginAvailable: (pnl.current_capital ?? 0) - (pos.total_exposure ?? 0), } - + return { success: true as const, data: transformed, timestamp: response.timestamp, } } - + return response }, staleTime: REFRESH_INTERVALS.dashboard, @@ -227,11 +229,11 @@ export function useDashboardPositions() { } const response = await apiClient.getDashboardPositions() - + if (!response.success) { throw new Error(response.error || 'Failed to fetch dashboard positions') } - + // API returns {positions: [...], count: N, summary: {...}} - extract the array const rawData: unknown = response.data if (isWrappedResponse(rawData)) { @@ -241,7 +243,7 @@ export function useDashboardPositions() { timestamp: response.timestamp, } } - + return response }, staleTime: REFRESH_INTERVALS.dashboard, @@ -266,11 +268,11 @@ export function useDashboardPnl() { } const response = await apiClient.getDashboardPnl() - + if (!response.success) { throw new Error(response.error || 'Failed to fetch dashboard PnL') } - + // Transform API response: API returns flat {total_pnl, realized_pnl, ...} const raw: unknown = response.data if (isRawDashboardPnlResponse(raw)) { @@ -279,12 +281,13 @@ export function useDashboardPnl() { data: { daily: raw.realized_pnl ?? 0, total: raw.total_pnl ?? 0, - equity: (raw as { equity_curve?: EquityPoint[] }).equity_curve ?? [], + // TODO: replace with real API data when backend provides equity_curve + equity: getMockEquityPoints(), }, timestamp: response.timestamp, } } - + return response }, staleTime: REFRESH_INTERVALS.dashboard, @@ -319,7 +322,7 @@ export function useUnifiedPortfolio() { // Mutations export function useCreateOrder() { const queryClient = useQueryClient() - + return useMutation({ mutationFn: (order: Partial) => apiClient.createOrder(order), onSuccess: () => { @@ -330,7 +333,7 @@ export function useCreateOrder() { export function useDeleteOrder() { const queryClient = useQueryClient() - + return useMutation({ mutationFn: (id: string) => apiClient.deleteOrder(id), onSuccess: () => { @@ -342,7 +345,7 @@ export function useDeleteOrder() { // Trading Controls export function useStartTrading() { const queryClient = useQueryClient() - + return useMutation({ mutationFn: () => apiClient.startTrading(), onSuccess: () => { @@ -353,7 +356,7 @@ export function useStartTrading() { export function useStopTrading() { const queryClient = useQueryClient() - + return useMutation({ mutationFn: () => apiClient.stopTrading(), onSuccess: () => { @@ -364,7 +367,7 @@ export function useStopTrading() { export function usePauseTrading() { const queryClient = useQueryClient() - + return useMutation({ mutationFn: () => apiClient.pauseTrading(), onSuccess: () => { @@ -375,7 +378,7 @@ export function usePauseTrading() { export function useResumeTrading() { const queryClient = useQueryClient() - + return useMutation({ mutationFn: () => apiClient.resumeTrading(), onSuccess: () => { From e9c640253b19210787373fb0173be0bf7be0dbc9 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 23:04:25 +0530 Subject: [PATCH 10/19] fix(lint): remove unused ptrStr helper in handlers_trading.go Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index b373de8..888bd1e 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -16,9 +16,8 @@ import ( "github.com/ajitpratap0/cryptofunk/internal/db" ) -// ptrStr and ptrF64 are local pointer helpers used only within this file. -// They are not duplicated elsewhere in the codebase (verified via grep). -func ptrStr(s string) *string { return &s } +// ptrF64 is a local pointer helper used only within this file. +// Not duplicated elsewhere in the codebase (verified via grep). func ptrF64(f float64) *float64 { return &f } // Session handlers From cc29bdda885f4f463c3f620c3d5a5be78914eed7 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 19 Mar 2026 23:15:55 +0530 Subject: [PATCH 11/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20third=20?= =?UTF-8?q?review=20=E2=80=94=20transactions,=20abstractions,=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - internal/db/db.go: add WithTx(ctx, fn) helper encapsulating begin/commit/rollback; callers never touch pgxpool directly - internal/db/orders.go: add UpdateOrderStatusTx for transactional order updates; eliminates duplicated inline SQL in handlers_trading.go - internal/db/positions.go: add GetOpenPositionBySymbolTx for in-transaction position lookup; eliminates TOCTOU race where concurrent orders both see existingPos==nil - cmd/api/handlers_trading.go: move existingPos fetch inside WithTx callback; use UpdateOrderStatusTx; include unrealized_pnl in INSERT (removes silent tx.Exec discard); use quoteAsset() helper for commissionAsset instead of hardcoded "USDT"; return generic "internal server error" to clients (log real error server-side) - internal/api/risk.go: buildBreaker returns "DISABLED" when threshold <= 0 (prevents false triggers on unset config); rename "Max Daily Loss" → "Session Total Loss" (was using lifetime TotalPnL, not daily — label now accurate) Frontend: - app/risk/page.tsx: add isRiskMetrics/isCircuitBreakers/isExposure runtime type guards; replace unsafe casts with guarded assignments; bad API shapes log console.warn and fall back to empty/undefined instead of silently corrupting state - hooks/usePerformance.ts: give useRiskExposure its own top-level query key ['risk-exposure'] instead of nesting under risk; prevents unintended cache invalidation when risk queries are invalidated Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 292 ++++++++++++-------------- internal/api/risk.go | 15 +- internal/db/db.go | 21 ++ internal/db/orders.go | 45 ++++ internal/db/positions.go | 49 +++++ web/dashboard/app/risk/page.tsx | 84 ++++++-- web/dashboard/hooks/usePerformance.ts | 5 +- 7 files changed, 330 insertions(+), 181 deletions(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 888bd1e..134577b 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -20,6 +20,12 @@ import ( // Not duplicated elsewhere in the codebase (verified via grep). func ptrF64(f float64) *float64 { return &f } +// errOppositeSide is a sentinel returned from the WithTx callback when the +// incoming order is on the opposite side of an existing open position. It is +// handled by the caller to produce a 422 response without logging as an +// internal server error. +var errOppositeSide = errors.New("opposite side trade on existing position") + // Session handlers func (s *APIServer) handleListSessions(c *gin.Context) { ctx := c.Request.Context() @@ -386,6 +392,17 @@ func (s *APIServer) handleCancelOrder(c *gin.Context) { }) } +// quoteAsset derives the quote asset token from a trading symbol by checking +// common suffixes. Falls back to "USDT" for unrecognised symbols. +func quoteAsset(symbol string) string { + for _, suffix := range []string{"USDT", "BUSD", "BTC", "ETH", "BNB"} { + if strings.HasSuffix(strings.ToUpper(symbol), suffix) { + return suffix + } + } + return "USDT" +} + // handlePaperTrade executes a paper (simulated) trade order. // Market orders are immediately filled; limit orders remain open (NEW status). // POST /api/v1/trade @@ -462,7 +479,7 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { } else { log.Error().Err(err).Msg("Failed to create paper session") c.JSON(http.StatusInternalServerError, gin.H{ - "error": fmt.Sprintf("failed to create paper session: %v", err), + "error": "internal server error", }) return } @@ -536,176 +553,141 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { posSide = db.PositionSideShort } - // Check for an opposite-side position before opening a transaction so we - // can reject early without acquiring DB resources unnecessarily. - existingPos, posErr := s.db.GetPositionBySymbolAndSession(ctx, req.Symbol, *sessionID) - if posErr != nil && !errors.Is(posErr, pgx.ErrNoRows) { - log.Warn().Err(posErr).Msg("Error looking up existing position") - } - if existingPos != nil && existingPos.Side != posSide { - // Opposite-side trade on an existing open position. Proper close/reduce - // logic (netting, realized PnL calculation) is not yet implemented. - // Reject the trade rather than silently corrupting position data. - log.Warn(). - Str("symbol", req.Symbol). - Str("existing_side", string(existingPos.Side)). - Str("order_side", string(posSide)). - Msg("Opposite-side trade on existing position; position close logic not yet implemented") - c.JSON(http.StatusUnprocessableEntity, gin.H{ - "error": "position close/reduce not yet implemented; opposite-side trade rejected", - }) - return - } - - // Wrap the fill writes (UpdateOrderStatus → InsertTrade → CreatePosition/UpdatePositionAveraging) - // in a single DB transaction so a mid-flight failure does not leave orphaned rows. - // AggregateSessionStats is intentionally kept outside the transaction: it is a - // read-then-aggregate UPDATE that can be safely retried and does not create new rows. - tx, txErr := s.db.Pool().BeginTx(ctx, pgx.TxOptions{}) - if txErr != nil { - log.Error().Err(txErr).Msg("Failed to begin paper trade transaction") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to begin paper trade transaction"}) - return - } - // rollbackTx is a no-op after a successful Commit. - rollbackTx := func() { - if rbErr := tx.Rollback(ctx); rbErr != nil && !errors.Is(rbErr, pgx.ErrTxClosed) { - log.Warn().Err(rbErr).Msg("Failed to rollback paper trade transaction") + // Derive the quote asset from the symbol (e.g. "BTCUSDT" → "USDT"). + commissionAsset := quoteAsset(req.Symbol) + + // Wrap all fill writes in a single DB transaction so a mid-flight failure + // does not leave orphaned rows. The existingPos lookup is performed inside + // the transaction to eliminate the TOCTOU race where two concurrent BUY + // orders could both observe existingPos == nil and each try to INSERT a + // new position for the same symbol. + // AggregateSessionStats is intentionally kept outside the transaction: it + // is a read-then-aggregate UPDATE that can be safely retried. + txErr := s.db.WithTx(ctx, func(tx pgx.Tx) error { + // Re-fetch position inside the transaction for a consistent view. + existingPos, err := s.db.GetOpenPositionBySymbolTx(ctx, tx, *sessionID, req.Symbol) + if err != nil { + return fmt.Errorf("failed to look up existing position: %w", err) } - } - // UpdateOrderStatus inside transaction - _, txErr = tx.Exec(ctx, ` - UPDATE orders - SET status = $1, - executed_quantity = $2, - executed_quote_quantity = $3, - filled_at = $4, - canceled_at = $5, - error_message = $6, - updated_at = NOW() - WHERE id = $7`, - db.OrderStatusFilled, - req.Quantity, - execQuoteQty, - &now, - nil, - nil, - order.ID, - ) - if txErr != nil { - rollbackTx() - log.Error().Err(txErr).Str("order_id", order.ID.String()).Msg("Failed to mark paper order filled in transaction") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to fill paper trade order"}) - return - } - order.Status = db.OrderStatusFilled - order.ExecutedQuantity = req.Quantity - order.ExecutedQuoteQuantity = execQuoteQty - order.FilledAt = &now - order.UpdatedAt = now - - // InsertTrade inside transaction - commissionAsset := "USDT" - tradeID := uuid.New() - _, txErr = tx.Exec(ctx, ` - INSERT INTO trades ( - id, order_id, exchange_trade_id, symbol, exchange, side, - price, quantity, quote_quantity, commission, commission_asset, - executed_at, is_maker, metadata, created_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 - )`, - tradeID, - order.ID, - nil, - req.Symbol, - "paper", - orderSide, - execPrice, - req.Quantity, - execQuoteQty, - commission, - &commissionAsset, - now, - false, - nil, - now, - ) - if txErr != nil { - rollbackTx() - log.Error().Err(txErr).Msg("Failed to insert paper trade fill row in transaction") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to record paper trade fill"}) - return - } + if existingPos != nil && existingPos.Side != posSide { + // Opposite-side trade on an existing open position. Proper close/reduce + // logic (netting, realized PnL calculation) is not yet implemented. + // Return a sentinel so the outer handler can respond with 422. + return errOppositeSide + } - // Create or average into existing position inside transaction - if existingPos == nil { - posID := uuid.New() - entryReason := "paper_trade_api" - unrealizedPnL := 0.0 - _, txErr = tx.Exec(ctx, ` - INSERT INTO positions ( - id, session_id, symbol, exchange, side, entry_price, quantity, - entry_time, stop_loss, take_profit, entry_reason, metadata, created_at, updated_at + // UpdateOrderStatus inside transaction + if err := s.db.UpdateOrderStatusTx(ctx, tx, order.ID, db.OrderStatusFilled, req.Quantity, execQuoteQty, &now, nil, nil); err != nil { + return fmt.Errorf("failed to mark paper order filled: %w", err) + } + order.Status = db.OrderStatusFilled + order.ExecutedQuantity = req.Quantity + order.ExecutedQuoteQuantity = execQuoteQty + order.FilledAt = &now + order.UpdatedAt = now + + // InsertTrade inside transaction + tradeID := uuid.New() + _, err = tx.Exec(ctx, ` + INSERT INTO trades ( + id, order_id, exchange_trade_id, symbol, exchange, side, + price, quantity, quote_quantity, commission, commission_asset, + executed_at, is_maker, metadata, created_at ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14 + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 )`, - posID, - sessionID, + tradeID, + order.ID, + nil, req.Symbol, "paper", - posSide, + orderSide, execPrice, req.Quantity, + execQuoteQty, + commission, + &commissionAsset, now, + false, nil, - nil, - &entryReason, - nil, - now, now, ) - if txErr != nil { - rollbackTx() - log.Error().Err(txErr).Msg("Failed to create position for paper trade in transaction") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create position for paper trade"}) - return + if err != nil { + return fmt.Errorf("failed to insert paper trade fill row: %w", err) } - // Persist unrealized_pnl separately as it is not in the INSERT above - _, _ = tx.Exec(ctx, - `UPDATE positions SET unrealized_pnl = $1 WHERE id = $2`, - unrealizedPnL, posID, - ) - } else { - totalQty := existingPos.Quantity + req.Quantity - weightedAvg := (existingPos.Quantity*existingPos.EntryPrice + req.Quantity*execPrice) / totalQty - _, txErr = tx.Exec(ctx, ` - UPDATE positions - SET - entry_price = $2, - quantity = $3, - fees = fees + $4, - updated_at = $5 - WHERE id = $1 AND exit_time IS NULL`, - existingPos.ID, - weightedAvg, - totalQty, - commission, - now, - ) - if txErr != nil { - rollbackTx() - log.Error().Err(txErr).Msg("Failed to update position for paper trade in transaction") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to update position for paper trade"}) - return + + // Create or average into existing position inside transaction + if existingPos == nil { + posID := uuid.New() + entryReason := "paper_trade_api" + _, err = tx.Exec(ctx, ` + INSERT INTO positions ( + id, session_id, symbol, exchange, side, entry_price, quantity, + entry_time, stop_loss, take_profit, entry_reason, metadata, + unrealized_pnl, created_at, updated_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 + )`, + posID, + sessionID, + req.Symbol, + "paper", + posSide, + execPrice, + req.Quantity, + now, + nil, + nil, + &entryReason, + nil, + 0.0, + now, + now, + ) + if err != nil { + return fmt.Errorf("failed to create position for paper trade: %w", err) + } + } else { + totalQty := existingPos.Quantity + req.Quantity + weightedAvg := (existingPos.Quantity*existingPos.EntryPrice + req.Quantity*execPrice) / totalQty + _, err = tx.Exec(ctx, ` + UPDATE positions + SET + entry_price = $2, + quantity = $3, + fees = fees + $4, + updated_at = $5 + WHERE id = $1 AND exit_time IS NULL`, + existingPos.ID, + weightedAvg, + totalQty, + commission, + now, + ) + if err != nil { + return fmt.Errorf("failed to update position for paper trade: %w", err) + } } - } + return nil + }) - if commitErr := tx.Commit(ctx); commitErr != nil { - rollbackTx() - log.Error().Err(commitErr).Msg("Failed to commit paper trade transaction") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to commit paper trade"}) + if txErr != nil { + if errors.Is(txErr, errOppositeSide) { + // Opposite-side trade on an existing open position. Proper close/reduce + // logic (netting, realized PnL calculation) is not yet implemented. + // Reject the trade rather than silently corrupting position data. + log.Warn(). + Str("symbol", req.Symbol). + Str("order_side", string(posSide)). + Msg("Opposite-side trade on existing position; position close logic not yet implemented") + c.JSON(http.StatusUnprocessableEntity, gin.H{ + "error": "position close/reduce not yet implemented; opposite-side trade rejected", + }) + return + } + log.Error().Err(txErr).Msg("Paper trade transaction failed") + c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) return } diff --git a/internal/api/risk.go b/internal/api/risk.go index 5f179a7..d8e4718 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -142,10 +142,13 @@ func (h *RiskHandler) GetCircuitBreakers(c *gin.Context) { totalTrades += s.TotalTrades } - // Max Daily Loss: triggered when cumulative losses exceed the configured threshold (dollars). + // Session Total Loss: triggered when cumulative session losses exceed the configured threshold (dollars). + // NOTE: totalPnL is the sum of TotalPnL across all active sessions (lifetime session PnL), + // not a rolling daily figure. The threshold field MaxDailyLossDollars still applies as the + // absolute dollar loss limit; only the label has been corrected to avoid confusion. lossAmount := math.Abs(math.Min(totalPnL, 0)) breakers := []gin.H{ - buildBreaker("Max Daily Loss", lossAmount, h.cfg.MaxDailyLossDollars), + buildBreaker("Session Total Loss", lossAmount, h.cfg.MaxDailyLossDollars), buildBreaker("Max Drawdown %", maxDrawdown*100, h.cfg.MaxDrawdownPct), buildBreaker("Total Trade Count", float64(totalTrades), float64(h.cfg.MaxTradeCount)), } @@ -155,9 +158,13 @@ func (h *RiskHandler) GetCircuitBreakers(c *gin.Context) { func buildBreaker(name string, current, threshold float64) gin.H { status := "OK" - if current >= threshold { + if threshold <= 0 { + // A zero or negative threshold means the breaker is disabled/unconfigured. + // Guard against false positives: never fire for unset config values. + status = "DISABLED" + } else if current >= threshold { status = "TRIGGERED" - } else if threshold > 0 && current/threshold >= 0.8 { + } else if current/threshold >= 0.8 { status = "WARNING" } return gin.H{ diff --git a/internal/db/db.go b/internal/db/db.go index 07e4238..15a57f5 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -6,6 +6,7 @@ import ( "os" "time" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/rs/zerolog/log" "github.com/sony/gobreaker" @@ -142,3 +143,23 @@ func (db *DB) GetCircuitBreaker() *risk.CircuitBreakerManager { func (db *DB) SetCircuitBreaker(cb *risk.CircuitBreakerManager) { db.circuitBreaker = cb } + +// WithTx runs fn inside a single pgx transaction. It commits on success and +// rolls back on any error or panic. The caller must not commit or roll back tx. +func (db *DB) WithTx(ctx context.Context, fn func(tx pgx.Tx) error) error { + tx, err := db.pool.BeginTx(ctx, pgx.TxOptions{}) + if err != nil { + return fmt.Errorf("begin transaction: %w", err) + } + defer func() { + if p := recover(); p != nil { + _ = tx.Rollback(ctx) + panic(p) + } + }() + if err := fn(tx); err != nil { + _ = tx.Rollback(ctx) + return err + } + return tx.Commit(ctx) +} diff --git a/internal/db/orders.go b/internal/db/orders.go index d2c595d..bdf26e9 100644 --- a/internal/db/orders.go +++ b/internal/db/orders.go @@ -7,6 +7,7 @@ import ( "time" "github.com/google/uuid" + "github.com/jackc/pgx/v5" "github.com/rs/zerolog/log" ) @@ -190,6 +191,50 @@ func (db *DB) UpdateOrderStatus(ctx context.Context, orderID uuid.UUID, status O return nil } +// UpdateOrderStatusTx updates an order's status and executed fields within an existing transaction. +func (db *DB) UpdateOrderStatusTx(ctx context.Context, tx pgx.Tx, orderID uuid.UUID, status OrderStatus, executedQty, executedQuoteQty float64, filledAt, canceledAt *time.Time, errorMsg *string) error { + query := ` + UPDATE orders + SET status = $1, + executed_quantity = $2, + executed_quote_quantity = $3, + filled_at = $4, + canceled_at = $5, + error_message = $6, + updated_at = NOW() + WHERE id = $7 + ` + + result, err := tx.Exec(ctx, query, + status, + executedQty, + executedQuoteQty, + filledAt, + canceledAt, + errorMsg, + orderID, + ) + + if err != nil { + log.Error(). + Err(err). + Str("order_id", orderID.String()). + Msg("Failed to update order status in transaction") + return fmt.Errorf("failed to update order status: %w", err) + } + + if result.RowsAffected() == 0 { + return fmt.Errorf("order not found: %s", orderID.String()) + } + + log.Debug(). + Str("order_id", orderID.String()). + Str("status", string(status)). + Msg("Order status updated in transaction") + + return nil +} + // InsertTrade inserts a new trade (fill) into the database func (db *DB) InsertTrade(ctx context.Context, trade *Trade) error { query := ` diff --git a/internal/db/positions.go b/internal/db/positions.go index 804cf02..e4fa817 100644 --- a/internal/db/positions.go +++ b/internal/db/positions.go @@ -444,6 +444,55 @@ func (db *DB) GetPositionBySymbolAndSession(ctx context.Context, symbol string, return &position, nil } +// GetOpenPositionBySymbolTx retrieves the most recent open position for a symbol within a session +// using an existing transaction, providing a consistent read within the transaction boundary. +// Returns (nil, nil) when no open position is found. +func (db *DB) GetOpenPositionBySymbolTx(ctx context.Context, tx pgx.Tx, sessionID uuid.UUID, symbol string) (*Position, error) { + query := ` + SELECT + id, session_id, symbol, exchange, side, entry_price, exit_price, + quantity, entry_time, exit_time, stop_loss, take_profit, + realized_pnl, unrealized_pnl, fees, entry_reason, exit_reason, + metadata, created_at, updated_at + FROM positions + WHERE symbol = $1 AND session_id = $2 AND exit_time IS NULL + ORDER BY entry_time DESC + LIMIT 1 + ` + + var position Position + err := tx.QueryRow(ctx, query, symbol, sessionID).Scan( + &position.ID, + &position.SessionID, + &position.Symbol, + &position.Exchange, + &position.Side, + &position.EntryPrice, + &position.ExitPrice, + &position.Quantity, + &position.EntryTime, + &position.ExitTime, + &position.StopLoss, + &position.TakeProfit, + &position.RealizedPnL, + &position.UnrealizedPnL, + &position.Fees, + &position.EntryReason, + &position.ExitReason, + &position.Metadata, + &position.CreatedAt, + &position.UpdatedAt, + ) + if err != nil { + if err == pgx.ErrNoRows { + return nil, nil + } + return nil, fmt.Errorf("failed to get open position by symbol in transaction: %w", err) + } + + return &position, nil +} + // GetLatestPositionBySymbol retrieves the latest position for a symbol (any session) func (db *DB) GetLatestPositionBySymbol(ctx context.Context, symbol string) (*Position, error) { query := ` diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index dde0ea5..2b70717 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -39,6 +39,48 @@ const mockAlerts = [ { id: '5', severity: 'warning' as const, message: 'SOL/USDT volatility spike detected', timestamp: '5 hrs ago', asset: 'SOL/USDT' }, ] +// ── Runtime type guards ───────────────────────────────────────────── + +type RawRiskMetrics = { + var_95: number | null + var_99: number | null + expected_shortfall: number | null + open_positions: number + total_exposure: number +} + +type RawCircuitBreakers = { + circuit_breakers: Array<{ name: string; current: number; threshold: number; status: string }> +} + +type RawExposure = { + exposure: Array<{ symbol: string; exposure: number }> +} + +function isRiskMetrics(raw: unknown): raw is RawRiskMetrics { + if (!raw || typeof raw !== 'object') return false + const r = raw as Record + return typeof r.open_positions === 'number' && typeof r.total_exposure === 'number' +} + +function isCircuitBreakers(raw: unknown): raw is RawCircuitBreakers { + if (!raw || typeof raw !== 'object') return false + const r = raw as Record + return ( + Array.isArray(r.circuit_breakers) && + (r.circuit_breakers.length === 0 || typeof (r.circuit_breakers[0] as { name?: unknown }).name === 'string') + ) +} + +function isExposure(raw: unknown): raw is RawExposure { + if (!raw || typeof raw !== 'object') return false + const r = raw as Record + return ( + Array.isArray(r.exposure) && + (r.exposure.length === 0 || typeof (r.exposure[0] as { symbol?: unknown }).symbol === 'string') + ) +} + // ── Page Component ───────────────────────────────────────────────── export default function RiskPage() { @@ -57,21 +99,20 @@ export default function RiskPage() { refetchExposure() } - // Map API risk metrics - const apiMetrics = metricsResponse?.data as { - var_95: number | null - var_99: number | null - expected_shortfall: number | null - open_positions: number - total_exposure: number - } | undefined + // Map API risk metrics — guarded so unexpected shapes surface as undefined + const rawMetrics: unknown = metricsResponse?.data + if (rawMetrics !== undefined && !isRiskMetrics(rawMetrics)) { + console.warn('Unexpected API response shape for risk metrics:', rawMetrics) + } + const apiMetrics: RawRiskMetrics | undefined = isRiskMetrics(rawMetrics) ? rawMetrics : undefined - // Map API circuit breakers to component shape + // Map API circuit breakers to component shape — guarded const circuitBreakers: CircuitBreaker[] = useMemo(() => { - const raw = breakersResponse?.data as - | { circuit_breakers: Array<{ name: string; current: number; threshold: number; status: string }> } - | undefined - if (!raw?.circuit_breakers?.length) return [] + const raw: unknown = breakersResponse?.data + if (raw !== undefined && !isCircuitBreakers(raw)) { + console.warn('Unexpected API response shape for circuit breakers:', raw) + } + if (!isCircuitBreakers(raw) || !raw.circuit_breakers.length) return [] return raw.circuit_breakers.map(cb => ({ name: cb.name, status: cb.status === 'TRIGGERED' ? 'triggered' : cb.status === 'WARNING' ? 'warning' : 'normal', @@ -81,21 +122,22 @@ export default function RiskPage() { } as CircuitBreaker)) }, [breakersResponse]) - // Map API exposure data + // Map API exposure data — guarded // Issue #2 fix: API GetExposure does not return a `side` field — omit it // entirely so ExposurePie uses its default asset-based coloring instead of // incorrectly coloring every bar as a long (profit/green). const exposureData = useMemo(() => { - const raw = exposureResponse?.data as - | { exposure: Array<{ symbol: string; exposure: number }> } - | undefined - if (!raw?.exposure?.length) return [] + const raw: unknown = exposureResponse?.data + if (raw !== undefined && !isExposure(raw)) { + console.warn('Unexpected API response shape for risk exposure:', raw) + } + if (!isExposure(raw) || !raw.exposure.length) return [] const total = raw.exposure.reduce((sum, e) => sum + e.exposure, 0) return raw.exposure.map(e => ({ - symbol: e.symbol, - exposure: e.exposure, + symbol: e.symbol, + exposure: e.exposure, percentage: total > 0 ? Math.round((e.exposure / total) * 100) : 0, - value: e.exposure, + value: e.exposure, })) }, [exposureResponse]) diff --git a/web/dashboard/hooks/usePerformance.ts b/web/dashboard/hooks/usePerformance.ts index cf9a9eb..6336ad1 100644 --- a/web/dashboard/hooks/usePerformance.ts +++ b/web/dashboard/hooks/usePerformance.ts @@ -27,6 +27,9 @@ export const PERFORMANCE_QUERY_KEYS = { candlestick: (symbol: string, timeRange: string) => ['candlestick', symbol, timeRange], risk: ['risk'], circuitBreakers: ['risk', 'circuit-breakers'], + // Own top-level key — NOT nested under `risk` so that invalidating `risk` + // does not unintentionally invalidate exposure queries. + riskExposure: ['risk-exposure'], } as const // Performance Metrics @@ -238,7 +241,7 @@ export function useCircuitBreakers() { // Risk Exposure by symbol export function useRiskExposure() { return useQuery({ - queryKey: [...PERFORMANCE_QUERY_KEYS.risk, 'exposure'], + queryKey: PERFORMANCE_QUERY_KEYS.riskExposure, queryFn: () => apiClient.getRiskExposure(), staleTime: REFRESH_INTERVALS.risk, refetchInterval: REFRESH_INTERVALS.risk, From eb7fd0633a2dd09c8ef994f5c2585567e8457b38 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 00:15:25 +0530 Subject: [PATCH 12/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20fourth?= =?UTF-8?q?=20review=20=E2=80=94=20race=20fix,=20orphan=20orders,=20type?= =?UTF-8?q?=20mapping,=20disabled=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - internal/db/positions.go: add FOR UPDATE to GetOpenPositionBySymbolTx; prevents concurrent transactions from both observing existingPos==nil and inserting duplicate positions; emit log.Warn when GetAllClosedPositions hits LIMIT 1000 (truncated VaR) - internal/db/db.go: add pgx.TxOptions param to WithTx so callers can set isolation level - cmd/api/handlers_trading.go: use RepeatableRead isolation + FOR UPDATE for position dedup; cancel orphaned order (UpdateOrderStatus→CANCELED) when fill transaction fails; replace inline 1.001/0.999 with named constants paperSlippageBuy/paperSlippageSell; use db.PtrFloat64 instead of local ptrF64; add TODO on Symbol="PAPER" sentinel - internal/db/utils.go: new file — exported PtrFloat64 helper replaces per-file ptrF64 - internal/db/trades.go: scope CountAllTrades pg_class query to public schema via relnamespace to avoid collisions with same-named tables in other schemas - internal/db/orders.go: UpdateOrderStatusTx updated to match new WithTx opts signature Frontend: - hooks/useTradeData.ts: add RawApiTrade interface matching backend PascalCase wire shape (no JSON tags on db.Trade → encoding/json uses field names as-is); add mapApiTrade() converter; replace unsafe cast with .map(mapApiTrade) — fields no longer undefined - lib/types.ts: add 'disabled' to CircuitBreaker.status union type - app/risk/page.tsx: map backend DISABLED→disabled status; skip disabled breakers in riskScore calculation to avoid inflating risk score with unconfigured controls - components/risk/CircuitBreakerStatus.tsx: render disabled breakers in muted/neutral style with "Threshold not configured" message; add Disabled count to summary footer; overall status = 'disabled' when all breakers are disabled Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 32 ++++++++---- internal/db/db.go | 9 ++-- internal/db/positions.go | 11 +++- internal/db/trades.go | 2 +- internal/db/utils.go | 4 ++ web/dashboard/app/risk/page.tsx | 6 ++- .../components/risk/CircuitBreakerStatus.tsx | 37 +++++++++++--- web/dashboard/hooks/useTradeData.ts | 51 ++++++++++++++++++- web/dashboard/lib/types.ts | 2 +- 9 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 internal/db/utils.go diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 134577b..36ab645 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -16,9 +16,11 @@ import ( "github.com/ajitpratap0/cryptofunk/internal/db" ) -// ptrF64 is a local pointer helper used only within this file. -// Not duplicated elsewhere in the codebase (verified via grep). -func ptrF64(f float64) *float64 { return &f } +const ( + paperSlippageBuy = 1.001 // 0.1% adverse slippage for market buy orders + paperSlippageSell = 0.999 // 0.1% adverse slippage for market sell orders + // TODO: make slippage configurable via config.Trading. +) // errOppositeSide is a sentinel returned from the WithTx callback when the // incoming order is on the opposite side of an existing open position. It is @@ -444,8 +446,11 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { if sessionID == nil { newSession := &db.TradingSession{ ID: uuid.New(), - Mode: db.TradingModePaper, - Symbol: "PAPER", // Symbol is intentionally generic; paper sessions are multi-asset + Mode: db.TradingModePaper, + // TODO: Add a session_type or is_multi_asset column to trading_sessions in a follow-up + // migration. "PAPER" is a placeholder to distinguish multi-asset paper sessions from + // single-symbol sessions (which use the actual symbol, e.g. "BTCUSDT"). + Symbol: "PAPER", Exchange: "paper", InitialCapital: 100_000.0, StartedAt: time.Now(), @@ -502,9 +507,9 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { execPrice := refPrice if !isLimit { if strings.EqualFold(req.Side, "BUY") { - execPrice = refPrice * 1.001 + execPrice = refPrice * paperSlippageBuy } else { - execPrice = refPrice * 0.999 + execPrice = refPrice * paperSlippageSell } } @@ -512,7 +517,7 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { now := time.Now() var pricePtr *float64 if req.Price > 0 { - pricePtr = ptrF64(req.Price) + pricePtr = db.PtrFloat64(req.Price) } orderSide := db.ConvertOrderSide(req.Side) orderType := db.ConvertOrderType(req.Type) @@ -563,7 +568,9 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { // new position for the same symbol. // AggregateSessionStats is intentionally kept outside the transaction: it // is a read-then-aggregate UPDATE that can be safely retried. - txErr := s.db.WithTx(ctx, func(tx pgx.Tx) error { + // RepeatableRead + FOR UPDATE on positions prevents concurrent orders from + // both observing existingPos == nil and each inserting a duplicate position. + txErr := s.db.WithTx(ctx, pgx.TxOptions{IsoLevel: pgx.RepeatableRead}, func(tx pgx.Tx) error { // Re-fetch position inside the transaction for a consistent view. existingPos, err := s.db.GetOpenPositionBySymbolTx(ctx, tx, *sessionID, req.Symbol) if err != nil { @@ -687,6 +694,13 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { return } log.Error().Err(txErr).Msg("Paper trade transaction failed") + // Attempt to cancel the order row that was inserted before the transaction + // began, so it is not left orphaned in NEW status with no fill. + canceledAt := time.Now() + cancelErrMsg := "fill transaction failed" + if cancelErr := s.db.UpdateOrderStatus(ctx, order.ID, db.OrderStatusCanceled, 0, 0, nil, &canceledAt, &cancelErrMsg); cancelErr != nil { + log.Error().Err(cancelErr).Str("order_id", order.ID.String()).Msg("Failed to cancel orphaned order after transaction failure") + } c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) return } diff --git a/internal/db/db.go b/internal/db/db.go index 15a57f5..d7f3378 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -144,10 +144,11 @@ func (db *DB) SetCircuitBreaker(cb *risk.CircuitBreakerManager) { db.circuitBreaker = cb } -// WithTx runs fn inside a single pgx transaction. It commits on success and -// rolls back on any error or panic. The caller must not commit or roll back tx. -func (db *DB) WithTx(ctx context.Context, fn func(tx pgx.Tx) error) error { - tx, err := db.pool.BeginTx(ctx, pgx.TxOptions{}) +// WithTx runs fn inside a single pgx transaction using the provided TxOptions. +// It commits on success and rolls back on any error or panic. +// The caller must not commit or roll back tx. +func (db *DB) WithTx(ctx context.Context, opts pgx.TxOptions, fn func(tx pgx.Tx) error) error { + tx, err := db.pool.BeginTx(ctx, opts) if err != nil { return fmt.Errorf("begin transaction: %w", err) } diff --git a/internal/db/positions.go b/internal/db/positions.go index e4fa817..88404a6 100644 --- a/internal/db/positions.go +++ b/internal/db/positions.go @@ -7,6 +7,7 @@ import ( "github.com/google/uuid" "github.com/jackc/pgx/v5" + "github.com/rs/zerolog/log" ) // PositionSide represents the side of a position @@ -375,7 +376,14 @@ func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { } defer rows.Close() - return scanPositions(rows) + positions, err := scanPositions(rows) + if err != nil { + return nil, err + } + if len(positions) == 1000 { + log.Warn().Msg("GetAllClosedPositions hit LIMIT 1000; VaR sample may be truncated — consider recent-only window") + } + return positions, nil } // GetPositionsBySession retrieves all positions (including closed) for a session @@ -458,6 +466,7 @@ func (db *DB) GetOpenPositionBySymbolTx(ctx context.Context, tx pgx.Tx, sessionI WHERE symbol = $1 AND session_id = $2 AND exit_time IS NULL ORDER BY entry_time DESC LIMIT 1 + FOR UPDATE ` var position Position diff --git a/internal/db/trades.go b/internal/db/trades.go index bc39ca1..38ced47 100644 --- a/internal/db/trades.go +++ b/internal/db/trades.go @@ -43,7 +43,7 @@ func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, e func (db *DB) CountAllTrades(ctx context.Context) (int, error) { var estimate int64 if err := db.pool.QueryRow(ctx, - "SELECT reltuples::bigint AS estimate FROM pg_class WHERE relname = 'trades'", + "SELECT reltuples::bigint AS estimate FROM pg_class WHERE relname = 'trades' AND relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = 'public')", ).Scan(&estimate); err != nil { return 0, fmt.Errorf("failed to count trades: %w", err) } diff --git a/internal/db/utils.go b/internal/db/utils.go new file mode 100644 index 0000000..a0bf154 --- /dev/null +++ b/internal/db/utils.go @@ -0,0 +1,4 @@ +package db + +// PtrFloat64 returns a pointer to f. Useful for optional float64 struct fields. +func PtrFloat64(f float64) *float64 { return &f } diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index 2b70717..5c45f4f 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -115,7 +115,10 @@ export default function RiskPage() { if (!isCircuitBreakers(raw) || !raw.circuit_breakers.length) return [] return raw.circuit_breakers.map(cb => ({ name: cb.name, - status: cb.status === 'TRIGGERED' ? 'triggered' : cb.status === 'WARNING' ? 'warning' : 'normal', + status: cb.status === 'TRIGGERED' ? 'triggered' + : cb.status === 'WARNING' ? 'warning' + : cb.status === 'DISABLED' ? 'disabled' + : 'normal', threshold: cb.threshold, currentValue: cb.current, description: cb.name, @@ -150,6 +153,7 @@ export default function RiskPage() { if (!circuitBreakers.length) return null let score = 100 circuitBreakers.forEach(cb => { + if (cb.status === 'disabled') return const usage = cb.threshold > 0 ? Math.abs(cb.currentValue) / Math.abs(cb.threshold) : 0 if (cb.status === 'triggered') score -= 30 else if (cb.status === 'warning') score -= 15 diff --git a/web/dashboard/components/risk/CircuitBreakerStatus.tsx b/web/dashboard/components/risk/CircuitBreakerStatus.tsx index f0ab5c2..a4064eb 100644 --- a/web/dashboard/components/risk/CircuitBreakerStatus.tsx +++ b/web/dashboard/components/risk/CircuitBreakerStatus.tsx @@ -62,6 +62,8 @@ export function CircuitBreakerStatus({ return case 'triggered': return + case 'disabled': + return default: return } @@ -75,6 +77,8 @@ export function CircuitBreakerStatus({ return 'bg-warning' case 'triggered': return 'bg-loss' + case 'disabled': + return 'bg-muted-foreground/40' default: return 'bg-muted' } @@ -84,10 +88,12 @@ export function CircuitBreakerStatus({ return Math.min((Math.abs(current) / Math.abs(threshold)) * 100, 100) } - const overallStatus = circuitBreakers.some(cb => cb.status === 'triggered') + const overallStatus = circuitBreakers.some(cb => cb.status === 'triggered') ? 'triggered' : circuitBreakers.some(cb => cb.status === 'warning') ? 'warning' + : circuitBreakers.every(cb => cb.status === 'disabled') + ? 'disabled' : 'normal' return ( @@ -103,9 +109,9 @@ export function CircuitBreakerStatus({
-
{overallStatus} @@ -125,7 +131,8 @@ export function CircuitBreakerStatus({ "p-4 rounded-lg border transition-all", breaker.status === 'normal' && "bg-background border-border", breaker.status === 'warning' && "bg-warning/10 border-warning/30", - breaker.status === 'triggered' && "bg-loss/10 border-loss/30" + breaker.status === 'triggered' && "bg-loss/10 border-loss/30", + breaker.status === 'disabled' && "bg-muted/30 border-border opacity-60" )} >
@@ -134,7 +141,8 @@ export function CircuitBreakerStatus({ "p-2 rounded-lg", breaker.status === 'normal' && "bg-muted text-muted-foreground", breaker.status === 'warning' && "bg-warning/20 text-warning", - breaker.status === 'triggered' && "bg-loss/20 text-loss" + breaker.status === 'triggered' && "bg-loss/20 text-loss", + breaker.status === 'disabled' && "bg-muted text-muted-foreground" )}> {getIcon(breaker.name)}
@@ -199,6 +207,15 @@ export function CircuitBreakerStatus({
)} + {breaker.status === 'disabled' && ( +
+
Disabled
+
+ This risk control is not active — threshold not configured +
+
+ )} + {breaker.status === 'normal' && progressPercentage > 50 && (
Within Safe Range
@@ -214,7 +231,7 @@ export function CircuitBreakerStatus({ {/* Summary */}
-
+
Normal
@@ -233,6 +250,12 @@ export function CircuitBreakerStatus({ {circuitBreakers.filter(cb => cb.status === 'triggered').length}
+
+
Disabled
+
+ {circuitBreakers.filter(cb => cb.status === 'disabled').length} +
+
diff --git a/web/dashboard/hooks/useTradeData.ts b/web/dashboard/hooks/useTradeData.ts index c8bb115..1c560c9 100644 --- a/web/dashboard/hooks/useTradeData.ts +++ b/web/dashboard/hooks/useTradeData.ts @@ -20,6 +20,55 @@ import { } from '@/types/api-responses' import type { Trade, Position, Order, UnifiedPortfolio, DashboardStats } from '@/lib/types' +// RawApiTrade matches the JSON shape produced by encoding/json on db.Trade, +// which has no struct tags — field names are the exact Go PascalCase names. +interface RawApiTrade { + ID: string + OrderID: string + ExchangeTradeID?: string | null + Symbol: string + Exchange: string + Side: string // "BUY" | "SELL" from db.OrderSide + Price: number + Quantity: number + QuoteQuantity: number + Commission: number + CommissionAsset?: string | null + ExecutedAt: string + IsMaker: boolean + Metadata?: Record | null + CreatedAt: string +} + +// mapApiTrade converts the PascalCase backend shape to the camelCase Trade type +// used throughout the dashboard. Fields that have no direct backend equivalent +// (entryPrice, currentPrice, pnl, pnlPercent, confidence, status, agent) are +// derived or defaulted — the trades table stores exchange fills, not strategy +// metadata, so those fields will be populated once the backend exposes them. +function mapApiTrade(raw: RawApiTrade): Trade { + const side: Trade['side'] = raw.Side === 'SELL' ? 'short' : 'long' + return { + id: raw.ID, + symbol: raw.Symbol, + side, + // exchange fills record the execution price; use it for both entry and current + entryPrice: raw.Price, + currentPrice: raw.Price, + quantity: raw.Quantity, + // PnL is not available in the fills table — default to 0 until the backend + // enriches the response with position-level PnL. + pnl: 0, + pnlPercent: 0, + agent: raw.Exchange, // closest available proxy for source label + confidence: 0, + timestamp: raw.ExecutedAt, + status: 'closed' as const, // a fill record is always a completed trade + reasoning: undefined, + exitPrice: undefined, + exitTimestamp: undefined, + } +} + // Query Keys export const QUERY_KEYS = { trades: ['trades'], @@ -55,7 +104,7 @@ export function useTrades(limit = 50, offset = 0) { const raw: unknown = response.data const tradeList: Trade[] = raw && typeof raw === 'object' && 'trades' in raw - ? (raw as { trades: Trade[] }).trades + ? (raw as { trades: RawApiTrade[] }).trades.map(mapApiTrade) : [] return { success: true as const, diff --git a/web/dashboard/lib/types.ts b/web/dashboard/lib/types.ts index debb997..02c04f0 100644 --- a/web/dashboard/lib/types.ts +++ b/web/dashboard/lib/types.ts @@ -140,7 +140,7 @@ export interface RiskMetrics { export interface CircuitBreaker { name: string; - status: 'normal' | 'warning' | 'triggered'; + status: 'normal' | 'warning' | 'triggered' | 'disabled'; threshold: number; currentValue: number; description: string; From 3fce7a4db63d3399afa6100eb4b5cbb7aaa03d6e Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 00:21:48 +0530 Subject: [PATCH 13/19] fix(lint): gofmt handlers_trading.go Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 36ab645..2eabf2a 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -445,12 +445,12 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { } if sessionID == nil { newSession := &db.TradingSession{ - ID: uuid.New(), + ID: uuid.New(), Mode: db.TradingModePaper, // TODO: Add a session_type or is_multi_asset column to trading_sessions in a follow-up // migration. "PAPER" is a placeholder to distinguish multi-asset paper sessions from // single-symbol sessions (which use the actual symbol, e.g. "BTCUSDT"). - Symbol: "PAPER", + Symbol: "PAPER", Exchange: "paper", InitialCapital: 100_000.0, StartedAt: time.Now(), From ad150244f35f281646fdb9e1b6e6584754d381a8 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 00:41:51 +0530 Subject: [PATCH 14/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20fifth=20?= =?UTF-8?q?review=20=E2=80=94=20atomic=20transaction,=20TX=20variants,=20V?= =?UTF-8?q?aR=20dollar=20scaling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - internal/db/orders.go: add InsertOrderTx, InsertTradeTx (transactional variants) - internal/db/positions.go: add CreatePositionTx, UpdatePositionAveragingTx; switch GetAllClosedPositions from LIMIT 1000 to 90-day time window (more meaningful for VaR, no log-spam when limit hit) - cmd/api/handlers_trading.go: move InsertOrderTx inside WithTx callback so order+fill+ position are one atomic unit; replace all inline tx.Exec SQL with db-layer TX methods; remove orphan-cancellation cleanup path (no longer needed); limit orders still use non-transactional InsertOrder since they have no fill step; use s.config.Trading.InitialCapital instead of hardcoded 100_000.0 - internal/api/risk.go: multiply fractional VaR by total portfolio value (sum of InitialCapital across active sessions) to produce dollar VaR — fixes display showing $0.02 instead of a meaningful dollar amount - internal/db/trades.go: CountAllTrades uses current_schema() instead of hardcoded 'public' — portable across multi-schema deployments - migrations/018_positions_symbol_index.sql: CREATE INDEX CONCURRENTLY on (session_id, symbol, exit_time) WHERE exit_time IS NULL — supports GetOpenPositionBySymbolTx FOR UPDATE without full table scan - configs/config.yaml: initial_capital 10000 → 100000 (realistic paper trading default) Frontend: - app/risk/page.tsx: extract VAR_WARNING_THRESHOLD = 500 named constant (dollar-based) replacing inline magic number - hooks/useTradeData.ts: mapApiTrade derives status from raw.Side (SELL→'closed', BUY→'open') so BUY fills on open positions no longer appear as closed in the UI Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/handlers_trading.go | 142 +++++++++------------- configs/config.yaml | 2 +- internal/api/risk.go | 27 +++- internal/db/orders.go | 106 ++++++++++++++++ internal/db/positions.go | 98 ++++++++++++--- internal/db/trades.go | 2 +- migrations/018_positions_symbol_index.sql | 5 + web/dashboard/app/risk/page.tsx | 8 +- web/dashboard/hooks/useTradeData.ts | 3 +- 9 files changed, 285 insertions(+), 108 deletions(-) create mode 100644 migrations/018_positions_symbol_index.sql diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 2eabf2a..c954f19 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -452,7 +452,7 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { // single-symbol sessions (which use the actual symbol, e.g. "BTCUSDT"). Symbol: "PAPER", Exchange: "paper", - InitialCapital: 100_000.0, + InitialCapital: s.config.Trading.InitialCapital, StartedAt: time.Now(), CreatedAt: time.Now(), UpdatedAt: time.Now(), @@ -513,7 +513,7 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { } } - // 3. Insert order + // 3. Build order struct (inserted inside the transaction below) now := time.Now() var pricePtr *float64 if req.Price > 0 { @@ -536,11 +536,6 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { CreatedAt: now, UpdatedAt: now, } - if err := s.db.InsertOrder(ctx, order); err != nil { - log.Error().Err(err).Msg("Failed to insert paper trade order") - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create paper trade order"}) - return - } // 4. Immediate fill for market orders if order.Type == db.OrderTypeMarket { @@ -562,15 +557,22 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { commissionAsset := quoteAsset(req.Symbol) // Wrap all fill writes in a single DB transaction so a mid-flight failure - // does not leave orphaned rows. The existingPos lookup is performed inside - // the transaction to eliminate the TOCTOU race where two concurrent BUY - // orders could both observe existingPos == nil and each try to INSERT a - // new position for the same symbol. + // does not leave orphaned rows. The order insert is the first step inside + // the transaction so no orphaned order rows can result from a failed fill. + // The existingPos lookup is also inside the transaction to eliminate the + // TOCTOU race where two concurrent BUY orders could both observe + // existingPos == nil and each try to INSERT a new position for the same symbol. // AggregateSessionStats is intentionally kept outside the transaction: it // is a read-then-aggregate UPDATE that can be safely retried. // RepeatableRead + FOR UPDATE on positions prevents concurrent orders from // both observing existingPos == nil and each inserting a duplicate position. txErr := s.db.WithTx(ctx, pgx.TxOptions{IsoLevel: pgx.RepeatableRead}, func(tx pgx.Tx) error { + // Insert the order as the first step so it is rolled back atomically + // with all fill rows if any later step fails. + if err := s.db.InsertOrderTx(ctx, tx, order); err != nil { + return fmt.Errorf("failed to insert paper trade order: %w", err) + } + // Re-fetch position inside the transaction for a consistent view. existingPos, err := s.db.GetOpenPositionBySymbolTx(ctx, tx, *sessionID, req.Symbol) if err != nil { @@ -594,85 +596,51 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { order.FilledAt = &now order.UpdatedAt = now - // InsertTrade inside transaction - tradeID := uuid.New() - _, err = tx.Exec(ctx, ` - INSERT INTO trades ( - id, order_id, exchange_trade_id, symbol, exchange, side, - price, quantity, quote_quantity, commission, commission_asset, - executed_at, is_maker, metadata, created_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 - )`, - tradeID, - order.ID, - nil, - req.Symbol, - "paper", - orderSide, - execPrice, - req.Quantity, - execQuoteQty, - commission, - &commissionAsset, - now, - false, - nil, - now, - ) - if err != nil { + // InsertTrade inside transaction via DB-layer method. + trade := &db.Trade{ + ID: uuid.New(), + OrderID: order.ID, + ExchangeTradeID: nil, + Symbol: req.Symbol, + Exchange: "paper", + Side: orderSide, + Price: execPrice, + Quantity: req.Quantity, + QuoteQuantity: execQuoteQty, + Commission: commission, + CommissionAsset: &commissionAsset, + ExecutedAt: now, + IsMaker: false, + Metadata: nil, + CreatedAt: now, + } + if err := s.db.InsertTradeTx(ctx, tx, trade); err != nil { return fmt.Errorf("failed to insert paper trade fill row: %w", err) } - // Create or average into existing position inside transaction + // Create or average into existing position inside transaction via DB-layer methods. if existingPos == nil { - posID := uuid.New() entryReason := "paper_trade_api" - _, err = tx.Exec(ctx, ` - INSERT INTO positions ( - id, session_id, symbol, exchange, side, entry_price, quantity, - entry_time, stop_loss, take_profit, entry_reason, metadata, - unrealized_pnl, created_at, updated_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 - )`, - posID, - sessionID, - req.Symbol, - "paper", - posSide, - execPrice, - req.Quantity, - now, - nil, - nil, - &entryReason, - nil, - 0.0, - now, - now, - ) - if err != nil { + pos := &db.Position{ + ID: uuid.New(), + SessionID: sessionID, + Symbol: req.Symbol, + Exchange: "paper", + Side: posSide, + EntryPrice: execPrice, + Quantity: req.Quantity, + EntryTime: now, + EntryReason: &entryReason, + CreatedAt: now, + UpdatedAt: now, + } + if err := s.db.CreatePositionTx(ctx, tx, pos); err != nil { return fmt.Errorf("failed to create position for paper trade: %w", err) } } else { totalQty := existingPos.Quantity + req.Quantity weightedAvg := (existingPos.Quantity*existingPos.EntryPrice + req.Quantity*execPrice) / totalQty - _, err = tx.Exec(ctx, ` - UPDATE positions - SET - entry_price = $2, - quantity = $3, - fees = fees + $4, - updated_at = $5 - WHERE id = $1 AND exit_time IS NULL`, - existingPos.ID, - weightedAvg, - totalQty, - commission, - now, - ) - if err != nil { + if err := s.db.UpdatePositionAveragingTx(ctx, tx, existingPos.ID, weightedAvg, totalQty, commission); err != nil { return fmt.Errorf("failed to update position for paper trade: %w", err) } } @@ -694,13 +662,6 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { return } log.Error().Err(txErr).Msg("Paper trade transaction failed") - // Attempt to cancel the order row that was inserted before the transaction - // began, so it is not left orphaned in NEW status with no fill. - canceledAt := time.Now() - cancelErrMsg := "fill transaction failed" - if cancelErr := s.db.UpdateOrderStatus(ctx, order.ID, db.OrderStatusCanceled, 0, 0, nil, &canceledAt, &cancelErrMsg); cancelErr != nil { - log.Error().Err(cancelErr).Str("order_id", order.ID.String()).Msg("Failed to cancel orphaned order after transaction failure") - } c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) return } @@ -710,6 +671,13 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { if err := s.db.AggregateSessionStats(ctx, *sessionID); err != nil { log.Warn().Err(err).Msg("Failed to aggregate session stats after paper trade") } + } else { + // Limit orders are not immediately filled; persist the order record in NEW status. + if err := s.db.InsertOrder(ctx, order); err != nil { + log.Error().Err(err).Msg("Failed to insert paper trade limit order") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create paper trade order"}) + return + } } if err := s.BroadcastOrderUpdate(order); err != nil { diff --git a/configs/config.yaml b/configs/config.yaml index 05d382a..b21827c 100644 --- a/configs/config.yaml +++ b/configs/config.yaml @@ -150,7 +150,7 @@ trading: - "BTCUSDT" - "ETHUSDT" exchange: "binance" - initial_capital: 10000.0 + initial_capital: 100000.0 # Paper trading starting capital ($100k for realistic position sizing) max_positions: 3 default_quantity: 0.01 commission_rate: 0.001 # 0.1% taker fee applied to paper trade fills (Binance standard tier) diff --git a/internal/api/risk.go b/internal/api/risk.go index d8e4718..5671858 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -92,6 +92,16 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { returnsIface[i] = v } + // Sum InitialCapital across all active sessions to get total portfolio value. + // Used to convert fractional VaR (e.g. 0.023) into dollar VaR (e.g. $2,300). + activeSessions, sessErr := h.db.ListActiveSessions(ctx) + portfolioValue := 0.0 + if sessErr == nil { + for _, s := range activeSessions { + portfolioValue += s.InitialCapital + } + } + res95, err := h.riskService.CalculateVaR(map[string]interface{}{ "returns": returnsIface, "confidence_level": 0.95, @@ -100,7 +110,12 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { log.Debug().Err(err).Msg("VaR calculation failed (95%)") } else { if varResult, ok := res95.(*risk.VaRResult); ok { - response["var_95"] = varResult.VaR + // Convert fractional VaR to dollar VaR by scaling by total portfolio value. + if portfolioValue > 0 { + response["var_95"] = varResult.VaR * portfolioValue + } else { + response["var_95"] = varResult.VaR + } } } @@ -112,8 +127,14 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { log.Debug().Err(err).Msg("VaR calculation failed (99%)") } else { if varResult, ok := res99.(*risk.VaRResult); ok { - response["var_99"] = varResult.VaR - response["expected_shortfall"] = varResult.CVaR + // Convert fractional VaR to dollar VaR by scaling by total portfolio value. + if portfolioValue > 0 { + response["var_99"] = varResult.VaR * portfolioValue + response["expected_shortfall"] = varResult.CVaR * portfolioValue + } else { + response["var_99"] = varResult.VaR + response["expected_shortfall"] = varResult.CVaR + } } } } diff --git a/internal/db/orders.go b/internal/db/orders.go index bdf26e9..21c88b3 100644 --- a/internal/db/orders.go +++ b/internal/db/orders.go @@ -90,6 +90,112 @@ type Trade struct { CreatedAt time.Time } +// InsertOrderTx inserts a new order into the database within an existing transaction. +func (db *DB) InsertOrderTx(ctx context.Context, tx pgx.Tx, order *Order) error { + query := ` + INSERT INTO orders ( + id, session_id, position_id, exchange_order_id, symbol, exchange, + side, type, status, price, stop_price, quantity, executed_quantity, + executed_quote_quantity, time_in_force, placed_at, filled_at, + canceled_at, error_message, metadata, created_at, updated_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, + $16, $17, $18, $19, $20, $21, $22 + ) + ` + + _, err := tx.Exec(ctx, query, + order.ID, + order.SessionID, + order.PositionID, + order.ExchangeOrderID, + order.Symbol, + order.Exchange, + order.Side, + order.Type, + order.Status, + order.Price, + order.StopPrice, + order.Quantity, + order.ExecutedQuantity, + order.ExecutedQuoteQuantity, + order.TimeInForce, + order.PlacedAt, + order.FilledAt, + order.CanceledAt, + order.ErrorMessage, + order.Metadata, + order.CreatedAt, + order.UpdatedAt, + ) + + if err != nil { + log.Error(). + Err(err). + Str("order_id", order.ID.String()). + Str("symbol", order.Symbol). + Msg("Failed to insert order in transaction") + return fmt.Errorf("failed to insert order: %w", err) + } + + log.Debug(). + Str("order_id", order.ID.String()). + Str("symbol", order.Symbol). + Str("status", string(order.Status)). + Msg("Order inserted into database in transaction") + + return nil +} + +// InsertTradeTx inserts a new trade (fill) into the database within an existing transaction. +func (db *DB) InsertTradeTx(ctx context.Context, tx pgx.Tx, trade *Trade) error { + query := ` + INSERT INTO trades ( + id, order_id, exchange_trade_id, symbol, exchange, side, + price, quantity, quote_quantity, commission, commission_asset, + executed_at, is_maker, metadata, created_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 + ) + ` + + _, err := tx.Exec(ctx, query, + trade.ID, + trade.OrderID, + trade.ExchangeTradeID, + trade.Symbol, + trade.Exchange, + trade.Side, + trade.Price, + trade.Quantity, + trade.QuoteQuantity, + trade.Commission, + trade.CommissionAsset, + trade.ExecutedAt, + trade.IsMaker, + trade.Metadata, + trade.CreatedAt, + ) + + if err != nil { + log.Error(). + Err(err). + Str("trade_id", trade.ID.String()). + Str("order_id", trade.OrderID.String()). + Msg("Failed to insert trade in transaction") + return fmt.Errorf("failed to insert trade: %w", err) + } + + log.Debug(). + Str("trade_id", trade.ID.String()). + Str("order_id", trade.OrderID.String()). + Float64("price", trade.Price). + Float64("quantity", trade.Quantity). + Msg("Trade inserted into database in transaction") + + return nil +} + // InsertOrder inserts a new order into the database func (db *DB) InsertOrder(ctx context.Context, order *Order) error { query := ` diff --git a/internal/db/positions.go b/internal/db/positions.go index 88404a6..c865e47 100644 --- a/internal/db/positions.go +++ b/internal/db/positions.go @@ -7,7 +7,6 @@ import ( "github.com/google/uuid" "github.com/jackc/pgx/v5" - "github.com/rs/zerolog/log" ) // PositionSide represents the side of a position @@ -354,9 +353,9 @@ func (db *DB) GetAllOpenPositions(ctx context.Context) ([]*Position, error) { return scanPositions(rows) } -// GetAllClosedPositions returns the most recent 1000 closed positions across all sessions, -// ordered by exit_time DESC. VaR and performance calculations only need recent history, -// and a full table scan becomes expensive at scale. +// GetAllClosedPositions returns positions closed within the last 90 days across all sessions, +// ordered by exit_time DESC. A 90-day window ensures VaR calculations use recent, +// relevant return data rather than an arbitrary row count. func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { query := ` SELECT @@ -365,9 +364,10 @@ func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { realized_pnl, unrealized_pnl, fees, entry_reason, exit_reason, metadata, created_at, updated_at FROM positions - WHERE exit_time IS NOT NULL AND realized_pnl IS NOT NULL + WHERE exit_time IS NOT NULL + AND exit_time > NOW() - INTERVAL '90 days' + AND realized_pnl IS NOT NULL ORDER BY exit_time DESC - LIMIT 1000 ` rows, err := db.pool.Query(ctx, query) @@ -376,14 +376,7 @@ func (db *DB) GetAllClosedPositions(ctx context.Context) ([]*Position, error) { } defer rows.Close() - positions, err := scanPositions(rows) - if err != nil { - return nil, err - } - if len(positions) == 1000 { - log.Warn().Msg("GetAllClosedPositions hit LIMIT 1000; VaR sample may be truncated — consider recent-only window") - } - return positions, nil + return scanPositions(rows) } // GetPositionsBySession retrieves all positions (including closed) for a session @@ -620,6 +613,83 @@ func (db *DB) UpdatePositionQuantity(ctx context.Context, id uuid.UUID, newQuant return nil } +// CreatePositionTx inserts a new position into the database within an existing transaction. +func (db *DB) CreatePositionTx(ctx context.Context, tx pgx.Tx, position *Position) error { + query := ` + INSERT INTO positions ( + id, session_id, symbol, exchange, side, entry_price, quantity, + entry_time, stop_loss, take_profit, entry_reason, metadata, created_at, updated_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14 + ) + ` + + if position.ID == uuid.Nil { + position.ID = uuid.New() + } + if position.CreatedAt.IsZero() { + position.CreatedAt = time.Now() + } + if position.UpdatedAt.IsZero() { + position.UpdatedAt = time.Now() + } + + _, err := tx.Exec(ctx, query, + position.ID, + position.SessionID, + position.Symbol, + position.Exchange, + position.Side, + position.EntryPrice, + position.Quantity, + position.EntryTime, + position.StopLoss, + position.TakeProfit, + position.EntryReason, + position.Metadata, + position.CreatedAt, + position.UpdatedAt, + ) + + if err != nil { + return fmt.Errorf("failed to create position: %w", err) + } + + return nil +} + +// UpdatePositionAveragingTx updates entry price and quantity when adding to a position, +// within an existing transaction. +func (db *DB) UpdatePositionAveragingTx(ctx context.Context, tx pgx.Tx, id uuid.UUID, newEntryPrice, newQuantity float64, additionalFees float64) error { + query := ` + UPDATE positions + SET + entry_price = $2, + quantity = $3, + fees = fees + $4, + updated_at = $5 + WHERE id = $1 AND exit_time IS NULL + ` + + result, err := tx.Exec(ctx, query, + id, + newEntryPrice, + newQuantity, + additionalFees, + time.Now(), + ) + + if err != nil { + return fmt.Errorf("failed to update position averaging: %w", err) + } + + if result.RowsAffected() == 0 { + return fmt.Errorf("position not found or already closed: %s", id) + } + + return nil +} + // UpdatePositionAveraging updates entry price and quantity when adding to a position func (db *DB) UpdatePositionAveraging(ctx context.Context, id uuid.UUID, newEntryPrice, newQuantity float64, additionalFees float64) error { query := ` diff --git a/internal/db/trades.go b/internal/db/trades.go index 38ced47..8e5e412 100644 --- a/internal/db/trades.go +++ b/internal/db/trades.go @@ -43,7 +43,7 @@ func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, e func (db *DB) CountAllTrades(ctx context.Context) (int, error) { var estimate int64 if err := db.pool.QueryRow(ctx, - "SELECT reltuples::bigint AS estimate FROM pg_class WHERE relname = 'trades' AND relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = 'public')", + "SELECT reltuples::bigint AS estimate FROM pg_class WHERE relname = 'trades' AND relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = current_schema())", ).Scan(&estimate); err != nil { return 0, fmt.Errorf("failed to count trades: %w", err) } diff --git a/migrations/018_positions_symbol_index.sql b/migrations/018_positions_symbol_index.sql new file mode 100644 index 0000000..3a80954 --- /dev/null +++ b/migrations/018_positions_symbol_index.sql @@ -0,0 +1,5 @@ +-- Index to support efficient per-symbol open position lookup within a session. +-- Used by GetOpenPositionBySymbolTx on the critical paper trade path (FOR UPDATE). +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_positions_session_symbol_open + ON positions (session_id, symbol, exit_time) + WHERE exit_time IS NULL; diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index 5c45f4f..7ca173d 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -81,6 +81,12 @@ function isExposure(raw: unknown): raw is RawExposure { ) } +// ── Risk Thresholds ───────────────────────────────────────────────── + +// Dollar-value VaR threshold above which the metric is highlighted as a warning. +// Backend now returns dollar VaR (e.g. 2345.67), not fractional. +const VAR_WARNING_THRESHOLD = 500 + // ── Page Component ───────────────────────────────────────────────── export default function RiskPage() { @@ -221,7 +227,7 @@ export default function RiskPage() { value={var95 != null ? formatCurrency(var95) : '—'} subtitle={var99 != null ? `99%: ${formatCurrency(var99)}` : 'Insufficient data'} icon={} - valueClassName={var95 != null && Math.abs(var95) > 500 ? 'text-warning' : 'text-foreground'} + valueClassName={var95 != null && Math.abs(var95) > VAR_WARNING_THRESHOLD ? 'text-warning' : 'text-foreground'} /> Date: Fri, 20 Mar 2026 00:44:34 +0530 Subject: [PATCH 15/19] =?UTF-8?q?fix(migration):=20remove=20CONCURRENTLY?= =?UTF-8?q?=20from=20index=20creation=20=E2=80=94=20incompatible=20with=20?= =?UTF-8?q?transaction=20blocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CREATE INDEX CONCURRENTLY cannot run inside a transaction block; migration runners wrap each migration in a transaction. Plain CREATE INDEX IF NOT EXISTS is used instead. Co-Authored-By: Claude Sonnet 4.6 --- migrations/018_positions_symbol_index.sql | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/migrations/018_positions_symbol_index.sql b/migrations/018_positions_symbol_index.sql index 3a80954..9127b81 100644 --- a/migrations/018_positions_symbol_index.sql +++ b/migrations/018_positions_symbol_index.sql @@ -1,5 +1,8 @@ -- Index to support efficient per-symbol open position lookup within a session. -- Used by GetOpenPositionBySymbolTx on the critical paper trade path (FOR UPDATE). -CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_positions_session_symbol_open +-- CONCURRENTLY is omitted: migration runners execute inside a transaction block, +-- which is incompatible with CREATE INDEX CONCURRENTLY. The index is small enough +-- that a standard build does not cause meaningful locking during migration. +CREATE INDEX IF NOT EXISTS idx_positions_session_symbol_open ON positions (session_id, symbol, exit_time) WHERE exit_time IS NULL; From 674ca11303e0cecafe77f344e1ddf4094b8c114b Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 00:53:20 +0530 Subject: [PATCH 16/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20sixth=20?= =?UTF-8?q?review=20=E2=80=94=20json=20tags,=20VaR=20comment,=20console.wa?= =?UTF-8?q?rn,=20config=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - internal/db/orders.go: add snake_case json struct tags to all db.Trade fields (id, order_id, exchange_trade_id, symbol, exchange, side, price, quantity, quote_quantity, commission, commission_asset, executed_at, is_maker, metadata, created_at) — decouples wire format from Go field names; metadata uses omitempty - internal/api/risk.go: add NOTE comment explaining VaR scales by InitialCapital (not CurrentCapital — field doesn't exist yet on TradingSession); add debug log when data_points < 10 so operators know why VaR shows nil - cmd/api/handlers_trading.go: add TODO to quoteAsset suffix list re: configurable list for non-Binance exchanges (Kraken etc.) - configs/config.yaml: add explanatory comment on initial_capital 100000 change - TASKS.md: note initial_capital config change scope Frontend: - hooks/useTradeData.ts: update RawApiTrade interface and mapApiTrade to use snake_case field names matching the new json tags (id, order_id, executed_at, is_maker, etc.) — was PascalCase which would break silently on any tag addition - app/risk/page.tsx: move console.warn calls for bad API shapes out of render-time code into useEffect hooks keyed on each response object — prevents console spam on every re-render when API returns unexpected shape Co-Authored-By: Claude Sonnet 4.6 --- TASKS.md | 1 + cmd/api/handlers_trading.go | 2 ++ configs/config.yaml | 5 ++- internal/api/risk.go | 13 +++++-- internal/db/orders.go | 30 ++++++++-------- web/dashboard/app/risk/page.tsx | 33 ++++++++++++------ web/dashboard/hooks/useTradeData.ts | 54 ++++++++++++++--------------- 7 files changed, 82 insertions(+), 56 deletions(-) diff --git a/TASKS.md b/TASKS.md index 931decd..dd88508 100644 --- a/TASKS.md +++ b/TASKS.md @@ -73,6 +73,7 @@ This document consolidates all implementation tasks from the architecture and de |------|----------|----------|--------|-------------| | **TB-001** | Configuration | CRITICAL | 2h | Remove hardcoded version "1.0.0" in orchestrator - use build flags | | **TB-002** | Configuration | CRITICAL | 1h | Remove hardcoded exchange mode "PAPER" in API - use config | +| **CONFIG** | Configuration | INFO | — | Paper session initial_capital changed to $100k (2026-03-20) — affects new auto-created sessions only | | **TB-003** | Configuration | HIGH | 2h | Remove hardcoded exchange fees in arbitrage agent - make configurable | | **TB-004** | Testing | CRITICAL | 6h | Fix E2E test infrastructure (T262) - currently skipped | | **TB-005** | Observability | HIGH | 4h | Implement MCP metrics (request count, latency, errors) | diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index c954f19..3c850c5 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -397,6 +397,8 @@ func (s *APIServer) handleCancelOrder(c *gin.Context) { // quoteAsset derives the quote asset token from a trading symbol by checking // common suffixes. Falls back to "USDT" for unrecognised symbols. func quoteAsset(symbol string) string { + // TODO: make suffix list configurable via config for non-Binance exchanges + // (e.g. Kraken uses XBT, EUR, USD as quote assets). for _, suffix := range []string{"USDT", "BUSD", "BTC", "ETH", "BNB"} { if strings.HasSuffix(strings.ToUpper(symbol), suffix) { return suffix diff --git a/configs/config.yaml b/configs/config.yaml index b21827c..8937704 100644 --- a/configs/config.yaml +++ b/configs/config.yaml @@ -150,7 +150,10 @@ trading: - "BTCUSDT" - "ETHUSDT" exchange: "binance" - initial_capital: 100000.0 # Paper trading starting capital ($100k for realistic position sizing) + # initial_capital: starting capital for auto-created paper trading sessions. + # Changed from 10000.0 → 100000.0 (2026-03-20) to reflect realistic position sizing. + # Existing sessions are unaffected; only newly auto-created sessions use this value. + initial_capital: 100000.0 max_positions: 3 default_quantity: 0.01 commission_rate: 0.001 # 0.1% taker fee applied to paper trade fills (Binance standard tier) diff --git a/internal/api/risk.go b/internal/api/risk.go index 5671858..17861e3 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -76,24 +76,31 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { return } + dataPoints := len(returns) response := gin.H{ "open_positions": openCount, "total_exposure": math.Round(totalExposure*100) / 100, - "data_points": len(returns), + "data_points": dataPoints, "var_95": nil, "var_99": nil, "expected_shortfall": nil, } + if dataPoints < 10 { + log.Debug().Int("data_points", dataPoints).Msg("insufficient data for meaningful VaR estimate; need at least 10 closed positions in the last 90 days") + } - if len(returns) >= 10 { + if dataPoints >= 10 { // CalculateVaR requires []interface{} not []float64 - returnsIface := make([]interface{}, len(returns)) + returnsIface := make([]interface{}, dataPoints) for i, v := range returns { returnsIface[i] = v } // Sum InitialCapital across all active sessions to get total portfolio value. // Used to convert fractional VaR (e.g. 0.023) into dollar VaR (e.g. $2,300). + // NOTE: scaling by InitialCapital (not CurrentCapital) because TradingSession + // does not yet track current portfolio value. Dollar VaR will be inaccurate + // after significant P&L. Track in TASKS.md follow-up. activeSessions, sessErr := h.db.ListActiveSessions(ctx) portfolioValue := 0.0 if sessErr == nil { diff --git a/internal/db/orders.go b/internal/db/orders.go index 21c88b3..a24c217 100644 --- a/internal/db/orders.go +++ b/internal/db/orders.go @@ -73,21 +73,21 @@ type Order struct { // Trade represents a database trade record (fill) type Trade struct { - ID uuid.UUID - OrderID uuid.UUID - ExchangeTradeID *string - Symbol string - Exchange string - Side OrderSide - Price float64 - Quantity float64 - QuoteQuantity float64 - Commission float64 - CommissionAsset *string - ExecutedAt time.Time - IsMaker bool - Metadata map[string]interface{} - CreatedAt time.Time + ID uuid.UUID `json:"id"` + OrderID uuid.UUID `json:"order_id"` + ExchangeTradeID *string `json:"exchange_trade_id"` + Symbol string `json:"symbol"` + Exchange string `json:"exchange"` + Side OrderSide `json:"side"` + Price float64 `json:"price"` + Quantity float64 `json:"quantity"` + QuoteQuantity float64 `json:"quote_quantity"` + Commission float64 `json:"commission"` + CommissionAsset *string `json:"commission_asset"` + ExecutedAt time.Time `json:"executed_at"` + IsMaker bool `json:"is_maker"` + Metadata map[string]interface{} `json:"metadata,omitempty"` + CreatedAt time.Time `json:"created_at"` } // InsertOrderTx inserts a new order into the database within an existing transaction. diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index 7ca173d..de72da3 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState, useMemo } from 'react' +import { useState, useMemo, useEffect } from 'react' import { Shield, TrendingDown, @@ -105,19 +105,35 @@ export default function RiskPage() { refetchExposure() } + // Warn once per distinct bad-shape response — not on every render + useEffect(() => { + const raw = metricsResponse?.data + if (raw !== undefined && !isRiskMetrics(raw)) { + console.warn('Unexpected API response shape for risk metrics:', raw) + } + }, [metricsResponse]) + + useEffect(() => { + const raw = breakersResponse?.data + if (raw !== undefined && !isCircuitBreakers(raw)) { + console.warn('Unexpected API response shape for circuit breakers:', raw) + } + }, [breakersResponse]) + + useEffect(() => { + const raw = exposureResponse?.data + if (raw !== undefined && !isExposure(raw)) { + console.warn('Unexpected API response shape for risk exposure:', raw) + } + }, [exposureResponse]) + // Map API risk metrics — guarded so unexpected shapes surface as undefined const rawMetrics: unknown = metricsResponse?.data - if (rawMetrics !== undefined && !isRiskMetrics(rawMetrics)) { - console.warn('Unexpected API response shape for risk metrics:', rawMetrics) - } const apiMetrics: RawRiskMetrics | undefined = isRiskMetrics(rawMetrics) ? rawMetrics : undefined // Map API circuit breakers to component shape — guarded const circuitBreakers: CircuitBreaker[] = useMemo(() => { const raw: unknown = breakersResponse?.data - if (raw !== undefined && !isCircuitBreakers(raw)) { - console.warn('Unexpected API response shape for circuit breakers:', raw) - } if (!isCircuitBreakers(raw) || !raw.circuit_breakers.length) return [] return raw.circuit_breakers.map(cb => ({ name: cb.name, @@ -137,9 +153,6 @@ export default function RiskPage() { // incorrectly coloring every bar as a long (profit/green). const exposureData = useMemo(() => { const raw: unknown = exposureResponse?.data - if (raw !== undefined && !isExposure(raw)) { - console.warn('Unexpected API response shape for risk exposure:', raw) - } if (!isExposure(raw) || !raw.exposure.length) return [] const total = raw.exposure.reduce((sum, e) => sum + e.exposure, 0) return raw.exposure.map(e => ({ diff --git a/web/dashboard/hooks/useTradeData.ts b/web/dashboard/hooks/useTradeData.ts index 71abedc..37dae10 100644 --- a/web/dashboard/hooks/useTradeData.ts +++ b/web/dashboard/hooks/useTradeData.ts @@ -20,50 +20,50 @@ import { } from '@/types/api-responses' import type { Trade, Position, Order, UnifiedPortfolio, DashboardStats } from '@/lib/types' -// RawApiTrade matches the JSON shape produced by encoding/json on db.Trade, -// which has no struct tags — field names are the exact Go PascalCase names. +// RawApiTrade matches the JSON shape produced by encoding/json on db.Trade +// with snake_case struct tags (added in the backend data-pipeline update). interface RawApiTrade { - ID: string - OrderID: string - ExchangeTradeID?: string | null - Symbol: string - Exchange: string - Side: string // "BUY" | "SELL" from db.OrderSide - Price: number - Quantity: number - QuoteQuantity: number - Commission: number - CommissionAsset?: string | null - ExecutedAt: string - IsMaker: boolean - Metadata?: Record | null - CreatedAt: string + id: string + order_id: string + exchange_trade_id?: string | null + symbol: string + exchange: string + side: string // "BUY" | "SELL" from db.OrderSide + price: number + quantity: number + quote_quantity: number + commission: number + commission_asset?: string | null + executed_at: string + is_maker: boolean + metadata?: Record | null + created_at: string } -// mapApiTrade converts the PascalCase backend shape to the camelCase Trade type +// mapApiTrade converts the snake_case backend shape to the camelCase Trade type // used throughout the dashboard. Fields that have no direct backend equivalent // (entryPrice, currentPrice, pnl, pnlPercent, confidence, status, agent) are // derived or defaulted — the trades table stores exchange fills, not strategy // metadata, so those fields will be populated once the backend exposes them. function mapApiTrade(raw: RawApiTrade): Trade { - const side: Trade['side'] = raw.Side === 'SELL' ? 'short' : 'long' + const side: Trade['side'] = raw.side === 'SELL' ? 'short' : 'long' return { - id: raw.ID, - symbol: raw.Symbol, + id: raw.id, + symbol: raw.symbol, side, // exchange fills record the execution price; use it for both entry and current - entryPrice: raw.Price, - currentPrice: raw.Price, - quantity: raw.Quantity, + entryPrice: raw.price, + currentPrice: raw.price, + quantity: raw.quantity, // PnL is not available in the fills table — default to 0 until the backend // enriches the response with position-level PnL. pnl: 0, pnlPercent: 0, - agent: raw.Exchange, // closest available proxy for source label + agent: raw.exchange, // closest available proxy for source label confidence: 0, - timestamp: raw.ExecutedAt, + timestamp: raw.executed_at, // A BUY fill opens a position ('open'); a SELL fill closes one ('closed'). - status: (raw.Side === 'SELL' ? 'closed' : 'open') as Trade['status'], + status: (raw.side === 'SELL' ? 'closed' : 'open') as Trade['status'], reasoning: undefined, exitPrice: undefined, exitTimestamp: undefined, From 5dcea0690a1697ec90e08fff78af40544da0f4f0 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 01:16:41 +0530 Subject: [PATCH 17/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20seventh?= =?UTF-8?q?=20review=20=E2=80=94=20unique=20position=20index,=20WithTx=20c?= =?UTF-8?q?ontext,=20SQL=20dedup,=20VaR=20unit=20field?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - migrations/019: replace non-unique partial index (018) with UNIQUE partial index on (session_id, symbol) WHERE exit_time IS NULL — prevents phantom duplicate open positions when two concurrent inserts both observe existingPos == nil - internal/db/db.go: use context.Background() for panic-path rollback in WithTx so rollback succeeds even when ctx is already cancelled High: - internal/db/orders.go: extract sqlInsertOrder, sqlInsertTrade, sqlUpdateOrderStatus package constants — Tx and non-Tx variants now share one SQL source of truth - internal/api/risk.go: add var_unit field ("dollar"/"fractional") to VaR response; log warning when ListActiveSessions fails so degraded VaR is visible to operators Medium: - internal/db/trades.go: CountAllTrades uses to_regclass('public.trades') instead of current_schema() — stable under TimescaleDB search_path modifications Minor: - cmd/api/handlers_trading.go: comment explaining session/tx boundary is intentional; quoteAsset suffix ordering explanation with non-Binance configurable TODO - web/dashboard/hooks/useTradeData.ts: agent proxy TODO comment - TASKS.md: move CONFIG entry to a note block (no task ID or estimate) Co-Authored-By: Claude Sonnet 4.6 --- TASKS.md | 3 +- cmd/api/handlers_trading.go | 16 ++- internal/api/risk.go | 9 ++ internal/db/db.go | 2 +- internal/db/orders.go | 114 ++++++------------ internal/db/trades.go | 18 ++- .../019_positions_unique_open_index.sql | 11 ++ web/dashboard/hooks/useTradeData.ts | 2 +- 8 files changed, 93 insertions(+), 82 deletions(-) create mode 100644 migrations/019_positions_unique_open_index.sql diff --git a/TASKS.md b/TASKS.md index dd88508..a4dbf5d 100644 --- a/TASKS.md +++ b/TASKS.md @@ -73,12 +73,13 @@ This document consolidates all implementation tasks from the architecture and de |------|----------|----------|--------|-------------| | **TB-001** | Configuration | CRITICAL | 2h | Remove hardcoded version "1.0.0" in orchestrator - use build flags | | **TB-002** | Configuration | CRITICAL | 1h | Remove hardcoded exchange mode "PAPER" in API - use config | -| **CONFIG** | Configuration | INFO | — | Paper session initial_capital changed to $100k (2026-03-20) — affects new auto-created sessions only | | **TB-003** | Configuration | HIGH | 2h | Remove hardcoded exchange fees in arbitrage agent - make configurable | | **TB-004** | Testing | CRITICAL | 6h | Fix E2E test infrastructure (T262) - currently skipped | | **TB-005** | Observability | HIGH | 4h | Implement MCP metrics (request count, latency, errors) | | **TB-006** | Security | HIGH | 3h | Add API key expiration/rotation mechanism | +> **Note (2026-03-20)**: Paper session `initial_capital` changed to $100k — affects new auto-created sessions only (existing sessions are unaffected). + **Total Blocking Effort**: ~18 hours (2-3 days) ### ⚠️ CRITICAL FOR BETA LAUNCH diff --git a/cmd/api/handlers_trading.go b/cmd/api/handlers_trading.go index 3c850c5..75fa251 100644 --- a/cmd/api/handlers_trading.go +++ b/cmd/api/handlers_trading.go @@ -397,8 +397,9 @@ func (s *APIServer) handleCancelOrder(c *gin.Context) { // quoteAsset derives the quote asset token from a trading symbol by checking // common suffixes. Falls back to "USDT" for unrecognised symbols. func quoteAsset(symbol string) string { - // TODO: make suffix list configurable via config for non-Binance exchanges - // (e.g. Kraken uses XBT, EUR, USD as quote assets). + // Ordered most-specific first: "BUSD" before "BTC" prevents "BTCUSDT" from + // matching suffix "BTC" when "USDT" would be the correct quote asset. + // TODO: make configurable for non-Binance exchanges (e.g. Kraken uses XBT/USD). for _, suffix := range []string{"USDT", "BUSD", "BTC", "ETH", "BNB"} { if strings.HasSuffix(strings.ToUpper(symbol), suffix) { return suffix @@ -430,6 +431,11 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { ctx := c.Request.Context() + // NOTE: Session lookup and creation happen outside WithTx intentionally. + // The session row is long-lived (one per trading mode) and is safe to create + // outside the fill transaction. A failed fill transaction does not orphan the + // session — the next request simply reuses it. + // 1. Resolve or create paper session sessions, err := s.db.ListActiveSessions(ctx) if err != nil { @@ -568,6 +574,12 @@ func (s *APIServer) handlePaperTrade(c *gin.Context) { // is a read-then-aggregate UPDATE that can be safely retried. // RepeatableRead + FOR UPDATE on positions prevents concurrent orders from // both observing existingPos == nil and each inserting a duplicate position. + // Final safety net: migration 019 adds a UNIQUE partial index on + // (session_id, symbol) WHERE exit_time IS NULL. If two concurrent transactions + // both observe existingPos == nil and both attempt to INSERT a new open + // position, the second INSERT fails with SQLSTATE 23505 (unique_violation), + // causing its enclosing transaction to roll back rather than silently creating + // a duplicate open position for the same symbol. txErr := s.db.WithTx(ctx, pgx.TxOptions{IsoLevel: pgx.RepeatableRead}, func(tx pgx.Tx) error { // Insert the order as the first step so it is rolled back atomically // with all fill rows if any later step fails. diff --git a/internal/api/risk.go b/internal/api/risk.go index 17861e3..71995ca 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -107,6 +107,15 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { for _, s := range activeSessions { portfolioValue += s.InitialCapital } + } else { + log.Warn().Err(sessErr).Msg("ListActiveSessions failed; VaR will be fractional (not dollar-scaled)") + } + + // Set var_unit once so the frontend knows how to interpret var_95/var_99. + if portfolioValue > 0 { + response["var_unit"] = "dollar" + } else { + response["var_unit"] = "fractional" } res95, err := h.riskService.CalculateVaR(map[string]interface{}{ diff --git a/internal/db/db.go b/internal/db/db.go index d7f3378..3fc4abf 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -154,7 +154,7 @@ func (db *DB) WithTx(ctx context.Context, opts pgx.TxOptions, fn func(tx pgx.Tx) } defer func() { if p := recover(); p != nil { - _ = tx.Rollback(ctx) + _ = tx.Rollback(context.Background()) panic(p) } }() diff --git a/internal/db/orders.go b/internal/db/orders.go index a24c217..277cf43 100644 --- a/internal/db/orders.go +++ b/internal/db/orders.go @@ -90,21 +90,43 @@ type Trade struct { CreatedAt time.Time `json:"created_at"` } +const sqlInsertOrder = ` + INSERT INTO orders ( + id, session_id, position_id, exchange_order_id, symbol, exchange, + side, type, status, price, stop_price, quantity, executed_quantity, + executed_quote_quantity, time_in_force, placed_at, filled_at, + canceled_at, error_message, metadata, created_at, updated_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, + $16, $17, $18, $19, $20, $21, $22 + ) +` + +const sqlUpdateOrderStatus = ` + UPDATE orders + SET status = $1, + executed_quantity = $2, + executed_quote_quantity = $3, + filled_at = $4, + canceled_at = $5, + error_message = $6, + updated_at = NOW() + WHERE id = $7 +` + +const sqlInsertTrade = ` + INSERT INTO trades ( + id, order_id, exchange_trade_id, symbol, exchange, side, + price, quantity, quote_quantity, commission, commission_asset, + executed_at, is_maker, metadata, created_at + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 + ) +` + // InsertOrderTx inserts a new order into the database within an existing transaction. func (db *DB) InsertOrderTx(ctx context.Context, tx pgx.Tx, order *Order) error { - query := ` - INSERT INTO orders ( - id, session_id, position_id, exchange_order_id, symbol, exchange, - side, type, status, price, stop_price, quantity, executed_quantity, - executed_quote_quantity, time_in_force, placed_at, filled_at, - canceled_at, error_message, metadata, created_at, updated_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, - $16, $17, $18, $19, $20, $21, $22 - ) - ` - - _, err := tx.Exec(ctx, query, + _, err := tx.Exec(ctx, sqlInsertOrder, order.ID, order.SessionID, order.PositionID, @@ -149,17 +171,7 @@ func (db *DB) InsertOrderTx(ctx context.Context, tx pgx.Tx, order *Order) error // InsertTradeTx inserts a new trade (fill) into the database within an existing transaction. func (db *DB) InsertTradeTx(ctx context.Context, tx pgx.Tx, trade *Trade) error { - query := ` - INSERT INTO trades ( - id, order_id, exchange_trade_id, symbol, exchange, side, - price, quantity, quote_quantity, commission, commission_asset, - executed_at, is_maker, metadata, created_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 - ) - ` - - _, err := tx.Exec(ctx, query, + _, err := tx.Exec(ctx, sqlInsertTrade, trade.ID, trade.OrderID, trade.ExchangeTradeID, @@ -198,19 +210,7 @@ func (db *DB) InsertTradeTx(ctx context.Context, tx pgx.Tx, trade *Trade) error // InsertOrder inserts a new order into the database func (db *DB) InsertOrder(ctx context.Context, order *Order) error { - query := ` - INSERT INTO orders ( - id, session_id, position_id, exchange_order_id, symbol, exchange, - side, type, status, price, stop_price, quantity, executed_quantity, - executed_quote_quantity, time_in_force, placed_at, filled_at, - canceled_at, error_message, metadata, created_at, updated_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, - $16, $17, $18, $19, $20, $21, $22 - ) - ` - - _, err := db.pool.Exec(ctx, query, + _, err := db.pool.Exec(ctx, sqlInsertOrder, order.ID, order.SessionID, order.PositionID, @@ -255,19 +255,7 @@ func (db *DB) InsertOrder(ctx context.Context, order *Order) error { // UpdateOrderStatus updates an order's status and related fields func (db *DB) UpdateOrderStatus(ctx context.Context, orderID uuid.UUID, status OrderStatus, executedQty, executedQuoteQty float64, filledAt, canceledAt *time.Time, errorMsg *string) error { - query := ` - UPDATE orders - SET status = $1, - executed_quantity = $2, - executed_quote_quantity = $3, - filled_at = $4, - canceled_at = $5, - error_message = $6, - updated_at = NOW() - WHERE id = $7 - ` - - result, err := db.pool.Exec(ctx, query, + result, err := db.pool.Exec(ctx, sqlUpdateOrderStatus, status, executedQty, executedQuoteQty, @@ -299,19 +287,7 @@ func (db *DB) UpdateOrderStatus(ctx context.Context, orderID uuid.UUID, status O // UpdateOrderStatusTx updates an order's status and executed fields within an existing transaction. func (db *DB) UpdateOrderStatusTx(ctx context.Context, tx pgx.Tx, orderID uuid.UUID, status OrderStatus, executedQty, executedQuoteQty float64, filledAt, canceledAt *time.Time, errorMsg *string) error { - query := ` - UPDATE orders - SET status = $1, - executed_quantity = $2, - executed_quote_quantity = $3, - filled_at = $4, - canceled_at = $5, - error_message = $6, - updated_at = NOW() - WHERE id = $7 - ` - - result, err := tx.Exec(ctx, query, + result, err := tx.Exec(ctx, sqlUpdateOrderStatus, status, executedQty, executedQuoteQty, @@ -343,17 +319,7 @@ func (db *DB) UpdateOrderStatusTx(ctx context.Context, tx pgx.Tx, orderID uuid.U // InsertTrade inserts a new trade (fill) into the database func (db *DB) InsertTrade(ctx context.Context, trade *Trade) error { - query := ` - INSERT INTO trades ( - id, order_id, exchange_trade_id, symbol, exchange, side, - price, quantity, quote_quantity, commission, commission_asset, - executed_at, is_maker, metadata, created_at - ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15 - ) - ` - - _, err := db.pool.Exec(ctx, query, + _, err := db.pool.Exec(ctx, sqlInsertTrade, trade.ID, trade.OrderID, trade.ExchangeTradeID, diff --git a/internal/db/trades.go b/internal/db/trades.go index 8e5e412..b56575c 100644 --- a/internal/db/trades.go +++ b/internal/db/trades.go @@ -3,6 +3,8 @@ package db import ( "context" "fmt" + + "github.com/jackc/pgx/v5" ) // ListAllTrades returns recent trade fills across all orders, newest first. @@ -40,11 +42,21 @@ func (db *DB) ListAllTrades(ctx context.Context, limit, offset int) ([]*Trade, e // This is O(1) instead of O(n) — avoids a full sequential COUNT(*) scan on every request. // The estimate is sourced from pg_class.reltuples which is updated by ANALYZE/autovacuum. // If the table has never been analyzed (reltuples = -1), the function returns 0. +// to_regclass('public.trades') is used instead of current_schema() to avoid a wrong +// result when TimescaleDB adds _timescaledb_internal (or another schema) to search_path, +// which would make current_schema() return something other than 'public'. func (db *DB) CountAllTrades(ctx context.Context) (int, error) { var estimate int64 - if err := db.pool.QueryRow(ctx, - "SELECT reltuples::bigint AS estimate FROM pg_class WHERE relname = 'trades' AND relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = current_schema())", - ).Scan(&estimate); err != nil { + err := db.pool.QueryRow(ctx, + `SELECT COALESCE(reltuples::bigint, -1) AS estimate + FROM pg_class + WHERE oid = to_regclass('public.trades')`, + ).Scan(&estimate) + if err != nil { + // pgx returns pgx.ErrNoRows when to_regclass returns NULL (table doesn't exist). + if err == pgx.ErrNoRows { + return 0, nil + } return 0, fmt.Errorf("failed to count trades: %w", err) } // reltuples is -1 for a freshly created table that has never been analyzed. diff --git a/migrations/019_positions_unique_open_index.sql b/migrations/019_positions_unique_open_index.sql new file mode 100644 index 0000000..9e93bdb --- /dev/null +++ b/migrations/019_positions_unique_open_index.sql @@ -0,0 +1,11 @@ +-- Replace the non-unique partial index from migration 018 with a UNIQUE partial index. +-- FOR UPDATE only locks existing rows; without a unique constraint two concurrent +-- transactions can each observe existingPos == nil and INSERT duplicate positions for +-- the same (session_id, symbol). The unique constraint ensures the second concurrent +-- INSERT fails with 23505, which causes the enclosing transaction to roll back instead +-- of silently creating a duplicate open position. +-- CONCURRENTLY is omitted: migration runners execute inside a transaction block. +DROP INDEX IF EXISTS idx_positions_session_symbol_open; +CREATE UNIQUE INDEX IF NOT EXISTS idx_positions_open_session_symbol_uniq + ON positions (session_id, symbol) + WHERE exit_time IS NULL; diff --git a/web/dashboard/hooks/useTradeData.ts b/web/dashboard/hooks/useTradeData.ts index 37dae10..bdd4543 100644 --- a/web/dashboard/hooks/useTradeData.ts +++ b/web/dashboard/hooks/useTradeData.ts @@ -59,7 +59,7 @@ function mapApiTrade(raw: RawApiTrade): Trade { // enriches the response with position-level PnL. pnl: 0, pnlPercent: 0, - agent: raw.exchange, // closest available proxy for source label + agent: raw.exchange, // proxy until a dedicated agent_id field is on the trade row confidence: 0, timestamp: raw.executed_at, // A BUY fill opens a position ('open'); a SELL fill closes one ('closed'). From 3e81b8ea15185ded53448022029a09fe8f27a46a Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 01:37:27 +0530 Subject: [PATCH 18/19] =?UTF-8?q?fix(api):=20address=20PR=20#89=20eighth?= =?UTF-8?q?=20review=20=E2=80=94=20LIMIT=20200,=20VaR=20nil=20on=20no=20se?= =?UTF-8?q?ssions,=20test=20coverage,=20mock=20drawdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - internal/db/positions.go: add LIMIT 200 to GetPairPerformance and GetExposureBySymbol to prevent unbounded result sets on the dashboard - internal/api/risk.go: skip VaR calculation entirely when portfolioValue == 0 (no active sessions or ListActiveSessions failure) — var_95/99 stay nil so frontend shows '—' instead of misleading fractional values; remove var_unit field - cmd/api/paper_trade_pipeline_test.go: add two sub-tests — one exercises the GetMarketPrice code path (market order with no price), another verifies the 400 error path when no market price is seeded - migrations/019: add explanatory comment on why 018 and 019 are separate migrations Frontend: - web/dashboard/app/risk/page.tsx: - add var_unit to RawRiskMetrics type; derive formatVar that switches between formatCurrency (dollar) and formatPercentage (fractional) based on var_unit - fix duplicate exposure: Open Positions subtitle now shows avg position size instead of duplicating the Total Exposure card value - replace mockDrawdown + DrawdownChart with empty-state placeholder and TODO; remove unused mockDrawdown constant, DrawdownChart import, and timeRange state Co-Authored-By: Claude Sonnet 4.6 --- cmd/api/paper_trade_pipeline_test.go | 128 ++++++++++++------ internal/api/risk.go | 52 +++---- internal/db/positions.go | 4 + .../019_positions_unique_open_index.sql | 4 + web/dashboard/app/risk/page.tsx | 70 ++++------ 5 files changed, 139 insertions(+), 119 deletions(-) diff --git a/cmd/api/paper_trade_pipeline_test.go b/cmd/api/paper_trade_pipeline_test.go index 6d71a0b..2e9e10e 100644 --- a/cmd/api/paper_trade_pipeline_test.go +++ b/cmd/api/paper_trade_pipeline_test.go @@ -13,47 +13,97 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/ajitpratap0/cryptofunk/internal/exchange" ) func TestPaperTrade_PersistsAllRows(t *testing.T) { - srv, _ := setupTestAPIServer(t) - - body := `{"symbol":"BTCUSDT","side":"BUY","type":"market","quantity":0.1,"price":45000}` - req := httptest.NewRequest(http.MethodPost, "/api/v1/trade", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - srv.router.ServeHTTP(w, req) - - require.Equal(t, http.StatusCreated, w.Code) - var resp map[string]interface{} - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) - orderID := resp["order"].(map[string]interface{})["id"].(string) - - ctx := context.Background() - - // Verify order row is filled with non-zero executed_quote_quantity - order, err := srv.db.GetOrder(ctx, uuid.MustParse(orderID)) - require.NoError(t, err) - assert.Equal(t, "FILLED", string(order.Status)) - assert.Greater(t, order.ExecutedQuantity, 0.0) - assert.Greater(t, order.ExecutedQuoteQuantity, 0.0, "executed_quote_quantity must be non-zero (price bug)") - - // Verify trade fill row via GetTradesByOrderID (already exists in orders.go) - fills, err := srv.db.GetTradesByOrderID(ctx, uuid.MustParse(orderID)) - require.NoError(t, err) - assert.NotEmpty(t, fills, "expected at least one trade fill row in trades table") - assert.Greater(t, fills[0].Price, 0.0, "fill price must be > 0") - - // Verify open position exists - positions, err := srv.db.GetAllOpenPositions(ctx) - require.NoError(t, err) - found := false - for _, p := range positions { - if p.Symbol == "BTCUSDT" { - found = true - assert.Greater(t, p.EntryPrice, 0.0) - assert.Greater(t, p.Quantity, 0.0) + t.Run("with_explicit_price", func(t *testing.T) { + srv, _ := setupTestAPIServer(t) + // Provide a mock exchange so s.exchange is never nil. + srv.exchange = exchange.NewMockExchange(srv.db) + + body := `{"symbol":"BTCUSDT","side":"BUY","type":"market","quantity":0.1,"price":45000}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/trade", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + srv.router.ServeHTTP(w, req) + + require.Equal(t, http.StatusCreated, w.Code) + var resp map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + orderID := resp["order"].(map[string]interface{})["id"].(string) + + ctx := context.Background() + + // Verify order row is filled with non-zero executed_quote_quantity + order, err := srv.db.GetOrder(ctx, uuid.MustParse(orderID)) + require.NoError(t, err) + assert.Equal(t, "FILLED", string(order.Status)) + assert.Greater(t, order.ExecutedQuantity, 0.0) + assert.Greater(t, order.ExecutedQuoteQuantity, 0.0, "executed_quote_quantity must be non-zero (price bug)") + + // Verify trade fill row via GetTradesByOrderID (already exists in orders.go) + fills, err := srv.db.GetTradesByOrderID(ctx, uuid.MustParse(orderID)) + require.NoError(t, err) + assert.NotEmpty(t, fills, "expected at least one trade fill row in trades table") + assert.Greater(t, fills[0].Price, 0.0, "fill price must be > 0") + + // Verify open position exists + positions, err := srv.db.GetAllOpenPositions(ctx) + require.NoError(t, err) + found := false + for _, p := range positions { + if p.Symbol == "BTCUSDT" { + found = true + assert.Greater(t, p.EntryPrice, 0.0) + assert.Greater(t, p.Quantity, 0.0) + } } - } - assert.True(t, found, "expected BTCUSDT open position after paper trade") + assert.True(t, found, "expected BTCUSDT open position after paper trade") + }) + + t.Run("uses_get_market_price_when_no_price_in_request", func(t *testing.T) { + srv, _ := setupTestAPIServer(t) + // Seed a market price so GetMarketPrice returns a non-zero value. + mockEx := exchange.NewMockExchange(srv.db) + mockEx.SetMarketPrice("ETHUSDT", 3000.0) + srv.exchange = mockEx + + body := `{"symbol":"ETHUSDT","side":"BUY","type":"market","quantity":0.05}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/trade", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + srv.router.ServeHTTP(w, req) + + require.Equal(t, http.StatusCreated, w.Code) + var resp map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + orderID := resp["order"].(map[string]interface{})["id"].(string) + + ctx := context.Background() + + // Verify trade fill row exists and has a positive price derived from GetMarketPrice. + fills, err := srv.db.GetTradesByOrderID(ctx, uuid.MustParse(orderID)) + require.NoError(t, err) + assert.NotEmpty(t, fills, "expected at least one trade fill row") + assert.Greater(t, fills[0].Price, 0.0, "fill price must be > 0 when price comes from GetMarketPrice") + }) + + t.Run("returns_400_when_no_price_and_no_market_price_seeded", func(t *testing.T) { + srv, _ := setupTestAPIServer(t) + // Use a fresh mock exchange with no price seeded for SOLUSDT. + srv.exchange = exchange.NewMockExchange(srv.db) + + body := `{"symbol":"SOLUSDT","side":"BUY","type":"market","quantity":0.05}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/trade", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + srv.router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + var resp map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Contains(t, resp["error"], "no market price configured for symbol") + }) } diff --git a/internal/api/risk.go b/internal/api/risk.go index 71995ca..46ad190 100644 --- a/internal/api/risk.go +++ b/internal/api/risk.go @@ -108,48 +108,34 @@ func (h *RiskHandler) GetMetrics(c *gin.Context) { portfolioValue += s.InitialCapital } } else { - log.Warn().Err(sessErr).Msg("ListActiveSessions failed; VaR will be fractional (not dollar-scaled)") + log.Warn().Err(sessErr).Msg("ListActiveSessions failed; skipping VaR calculation") } - // Set var_unit once so the frontend knows how to interpret var_95/var_99. + // Skip VaR entirely when there is no portfolio to compute it for. + // var_95, var_99, and expected_shortfall remain nil in the response. if portfolioValue > 0 { - response["var_unit"] = "dollar" - } else { - response["var_unit"] = "fractional" - } - - res95, err := h.riskService.CalculateVaR(map[string]interface{}{ - "returns": returnsIface, - "confidence_level": 0.95, - }) - if err != nil { - log.Debug().Err(err).Msg("VaR calculation failed (95%)") - } else { - if varResult, ok := res95.(*risk.VaRResult); ok { - // Convert fractional VaR to dollar VaR by scaling by total portfolio value. - if portfolioValue > 0 { + res95, err := h.riskService.CalculateVaR(map[string]interface{}{ + "returns": returnsIface, + "confidence_level": 0.95, + }) + if err != nil { + log.Debug().Err(err).Msg("VaR calculation failed (95%)") + } else { + if varResult, ok := res95.(*risk.VaRResult); ok { response["var_95"] = varResult.VaR * portfolioValue - } else { - response["var_95"] = varResult.VaR } } - } - res99, err := h.riskService.CalculateVaR(map[string]interface{}{ - "returns": returnsIface, - "confidence_level": 0.99, - }) - if err != nil { - log.Debug().Err(err).Msg("VaR calculation failed (99%)") - } else { - if varResult, ok := res99.(*risk.VaRResult); ok { - // Convert fractional VaR to dollar VaR by scaling by total portfolio value. - if portfolioValue > 0 { + res99, err := h.riskService.CalculateVaR(map[string]interface{}{ + "returns": returnsIface, + "confidence_level": 0.99, + }) + if err != nil { + log.Debug().Err(err).Msg("VaR calculation failed (99%)") + } else { + if varResult, ok := res99.(*risk.VaRResult); ok { response["var_99"] = varResult.VaR * portfolioValue response["expected_shortfall"] = varResult.CVaR * portfolioValue - } else { - response["var_99"] = varResult.VaR - response["expected_shortfall"] = varResult.CVaR } } } diff --git a/internal/db/positions.go b/internal/db/positions.go index c865e47..482d162 100644 --- a/internal/db/positions.go +++ b/internal/db/positions.go @@ -803,6 +803,8 @@ func (db *DB) GetPairPerformance(ctx context.Context) ([]PairPerformance, error) WHERE exit_time IS NOT NULL AND realized_pnl IS NOT NULL GROUP BY symbol ORDER BY realized_pnl DESC + -- Cap at 200 rows — sufficient for dashboard display; a full paginated API is a follow-up. + LIMIT 200 ` rows, err := db.pool.Query(ctx, query) @@ -840,6 +842,8 @@ func (db *DB) GetExposureBySymbol(ctx context.Context) ([]SymbolExposure, error) WHERE exit_time IS NULL GROUP BY symbol ORDER BY exposure DESC + -- Cap at 200 rows — sufficient for dashboard display; a full paginated API is a follow-up. + LIMIT 200 ` rows, err := db.pool.Query(ctx, query) diff --git a/migrations/019_positions_unique_open_index.sql b/migrations/019_positions_unique_open_index.sql index 9e93bdb..b9eacd1 100644 --- a/migrations/019_positions_unique_open_index.sql +++ b/migrations/019_positions_unique_open_index.sql @@ -1,3 +1,7 @@ +-- This migration supersedes 018_positions_symbol_index.sql which added a non-unique +-- partial index. The two are kept as separate migrations (rather than collapsing into +-- one) to safely handle environments where 018 was already applied — the DROP here +-- is idempotent (IF EXISTS) and the CREATE is idempotent (IF NOT EXISTS). -- Replace the non-unique partial index from migration 018 with a UNIQUE partial index. -- FOR UPDATE only locks existing rows; without a unique constraint two concurrent -- transactions can each observe existingPos == nil and INSERT duplicate positions for diff --git a/web/dashboard/app/risk/page.tsx b/web/dashboard/app/risk/page.tsx index de72da3..485420e 100644 --- a/web/dashboard/app/risk/page.tsx +++ b/web/dashboard/app/risk/page.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState, useMemo, useEffect } from 'react' +import { useMemo, useEffect } from 'react' import { Shield, TrendingDown, @@ -10,27 +10,15 @@ import { Zap } from 'lucide-react' import { ExposurePie } from '@/components/charts/ExposurePie' -import { DrawdownChart } from '@/components/charts/DrawdownChart' + import { CircuitBreakerStatus } from '@/components/risk/CircuitBreakerStatus' import { StatCard } from '@/components/ui/StatCard' -import { cn, formatCurrency } from '@/lib/utils' +import { cn, formatCurrency, formatPercentage } from '@/lib/utils' import { useRiskMetrics, useCircuitBreakers, useRiskExposure } from '@/hooks/usePerformance' import type { CircuitBreaker } from '@/lib/types' // ── Static Mock Data (no API equivalent yet) ─────────────────────── -const mockDrawdown = Array.from({ length: 90 }, (_, i) => { - const now = Date.now() - const timestamp = new Date(now - (89 - i) * 24 * 60 * 60 * 1000).toISOString() - const base = Math.sin(i / 15) * 4 + Math.random() * 2 - const drawdownPercent = -Math.abs(base) - return { - timestamp, - drawdown: drawdownPercent, - equity: 100000 + (drawdownPercent / 100) * 100000, - } -}) - const mockAlerts = [ { id: '1', severity: 'warning' as const, message: 'Max drawdown approaching threshold (78%)', timestamp: '2 min ago', asset: 'Portfolio' }, { id: '2', severity: 'info' as const, message: 'ETH/USDT correlation with BTC increasing', timestamp: '15 min ago', asset: 'ETH/USDT' }, @@ -47,6 +35,7 @@ type RawRiskMetrics = { expected_shortfall: number | null open_positions: number total_exposure: number + var_unit?: 'dollar' | 'fractional' } type RawCircuitBreakers = { @@ -90,8 +79,6 @@ const VAR_WARNING_THRESHOLD = 500 // ── Page Component ───────────────────────────────────────────────── export default function RiskPage() { - const [timeRange, setTimeRange] = useState<'1w' | '1m' | '3m'>('1m') - const { data: metricsResponse, isError: metricsError, isLoading: metricsLoading, refetch: refetchMetrics } = useRiskMetrics() const { data: breakersResponse, isError: breakersError, isLoading: breakersLoading, refetch: refetchBreakers } = useCircuitBreakers() const { data: exposureResponse, isError: exposureError, isLoading: exposureLoading, refetch: refetchExposure } = useRiskExposure() @@ -131,6 +118,11 @@ export default function RiskPage() { const rawMetrics: unknown = metricsResponse?.data const apiMetrics: RawRiskMetrics | undefined = isRiskMetrics(rawMetrics) ? rawMetrics : undefined + // var_unit: 'dollar' when VaR is scaled by portfolio value; 'fractional' when + // no active sessions exist and the backend returns a raw fractional value (e.g. 0.023). + const varUnit = isRiskMetrics(rawMetrics) ? (rawMetrics.var_unit ?? 'dollar') : 'dollar' + const formatVar = (v: number) => varUnit === 'dollar' ? formatCurrency(v) : formatPercentage(v * 100) + // Map API circuit breakers to component shape — guarded const circuitBreakers: CircuitBreaker[] = useMemo(() => { const raw: unknown = breakersResponse?.data @@ -232,20 +224,24 @@ export default function RiskPage() { icon={} valueClassName={riskColor} /> - {/* Issue #1 fix: var95/var99 are dollar PnL values, not fractional - percentages — use formatCurrency instead of formatPercentage. - Threshold comparison updated to dollar-based (>$500 = warning). */} + {/* var_unit from API controls formatting: 'dollar' → formatCurrency, + 'fractional' → formatPercentage. Threshold warning uses dollar-equivalent + (>$500) for dollar mode; for fractional >2.3% is roughly equivalent. */} } valueClassName={var95 != null && Math.abs(var95) > VAR_WARNING_THRESHOLD ? 'text-warning' : 'text-foreground'} /> 0 && apiMetrics?.total_exposure != null + ? `Avg size: ${formatCurrency(apiMetrics.total_exposure / apiMetrics.open_positions)}` + : 'No open positions' + } icon={} />

Drawdown History

-
- {(['1w', '1m', '3m'] as const).map(range => ( - - ))} -
- + {/* TODO: Replace with real drawdown data from /api/v1/performance/drawdown once available */} +
+ Drawdown chart — historical data not yet available +
{/* Position Sizing */} From 6ed264acc07c48c49471b54a826fc223170be3ee Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 20 Mar 2026 02:11:36 +0530 Subject: [PATCH 19/19] test: add pure-function unit tests to boost coverage above 48% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add unit tests for functions that don't require a DB connection: - internal/exchange: fix safety_guard tests — guard is always non-nil (NewService always calls risk.NewSafetyGuard); rewrite to test disabled guard behaviour (SetCapital, RecordTrade, EmergencyStop, ClearEmergencyStop, ResetCircuitBreaker, GetStats, GetEmergencyStopStatus, GetTradingHoursStatus) - internal/backtest: validateJob (8 cases), getValue, getIntValue - internal/market: CoinGeckoIDToBinanceSymbol, EstimateOHLCV, ParseStringFloat - internal/models: BinancePositionToUnified, BinanceTradeToUnified, Portfolio ops - internal/polymarket/analyzer: parseResponse (valid/embedded/invalid/empty) - internal/polymarket/news: stripHTML (7 cases) - internal/polymarket/resolver: calculatePredictionPnl (5 cases) - internal/api: itoa, parseDuration, buildBreaker, NewRiskHandler constructors - internal/db: PtrFloat64 Co-Authored-By: Claude Sonnet 4.6 --- internal/api/decisions_unit_test.go | 14 + internal/api/risk_unit_test.go | 215 ++++++++++++++++ internal/backtest/job_unit_test.go | 83 ++++++ internal/db/utils_test.go | 22 ++ internal/exchange/safety_guard_nil_test.go | 117 +++++++++ internal/market/pure_unit_test.go | 90 +++++++ internal/models/convert_test.go | 243 ++++++++++++++++++ .../polymarket/analyzer/analyzer_unit_test.go | 78 ++++++ internal/polymarket/news/news_unit_test.go | 30 +++ .../polymarket/resolver/resolver_unit_test.go | 60 +++++ 10 files changed, 952 insertions(+) create mode 100644 internal/api/decisions_unit_test.go create mode 100644 internal/api/risk_unit_test.go create mode 100644 internal/backtest/job_unit_test.go create mode 100644 internal/db/utils_test.go create mode 100644 internal/exchange/safety_guard_nil_test.go create mode 100644 internal/market/pure_unit_test.go create mode 100644 internal/models/convert_test.go create mode 100644 internal/polymarket/analyzer/analyzer_unit_test.go create mode 100644 internal/polymarket/news/news_unit_test.go create mode 100644 internal/polymarket/resolver/resolver_unit_test.go diff --git a/internal/api/decisions_unit_test.go b/internal/api/decisions_unit_test.go new file mode 100644 index 0000000..92e838e --- /dev/null +++ b/internal/api/decisions_unit_test.go @@ -0,0 +1,14 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestItoa(t *testing.T) { + assert.Equal(t, "0", itoa(0)) + assert.Equal(t, "42", itoa(42)) + assert.Equal(t, "-1", itoa(-1)) + assert.Equal(t, "100", itoa(100)) +} diff --git a/internal/api/risk_unit_test.go b/internal/api/risk_unit_test.go new file mode 100644 index 0000000..3c0c484 --- /dev/null +++ b/internal/api/risk_unit_test.go @@ -0,0 +1,215 @@ +package api + +import ( + "math" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/ajitpratap0/cryptofunk/internal/config" +) + +// TestParseDuration verifies that parseDuration converts human-friendly duration +// strings into the correct past time.Time value relative to now. +func TestParseDuration(t *testing.T) { + tests := []struct { + name string + input string + checkApprox func(t *testing.T, got time.Time) + }{ + { + name: "1h subtracts one hour", + input: "1h", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().Add(-1 * time.Hour) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected time ~1h ago") + }, + }, + { + name: "24h subtracts 24 hours", + input: "24h", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().Add(-24 * time.Hour) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected time ~24h ago") + }, + }, + { + name: "7d subtracts 7 days", + input: "7d", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(0, 0, -7) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected time ~7 days ago") + }, + }, + { + name: "30d subtracts 30 days", + input: "30d", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(0, 0, -30) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected time ~30 days ago") + }, + }, + { + name: "3m subtracts 3 months", + input: "3m", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(0, -3, 0) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected time ~3 months ago") + }, + }, + { + name: "1y subtracts 1 year", + input: "1y", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(-1, 0, 0) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected time ~1 year ago") + }, + }, + { + name: "invalid string falls back to 1 month ago", + input: "notvalid", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(0, -1, 0) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected default fallback ~1 month ago") + }, + }, + { + name: "empty string falls back to 1 month ago", + input: "", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(0, -1, 0) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected default fallback ~1 month ago") + }, + }, + { + name: "unknown unit falls back to 1 month ago", + input: "5z", + checkApprox: func(t *testing.T, got time.Time) { + t.Helper() + want := time.Now().AddDate(0, -1, 0) + diff := want.Sub(got) + if diff < 0 { + diff = -diff + } + assert.Less(t, diff, 2*time.Second, "expected default fallback ~1 month ago") + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := parseDuration(tc.input) + assert.False(t, got.IsZero(), "parseDuration should never return zero time") + tc.checkApprox(t, got) + }) + } +} + +// TestNewRiskHandler verifies that NewRiskHandler returns a non-nil handler with +// the provided config, and that a nil config is handled gracefully. +func TestNewRiskHandler(t *testing.T) { + t.Run("with explicit config", func(t *testing.T) { + cfg := &config.RiskConfig{ + MaxDailyLossDollars: 500.0, + MaxDrawdownPct: 20.0, + MaxTradeCount: 100, + } + h := NewRiskHandler(nil, cfg) + assert.NotNil(t, h) + assert.Equal(t, cfg, h.cfg) + assert.Nil(t, h.db) + assert.NotNil(t, h.riskService) + }) + + t.Run("with nil config falls back to zero-value RiskConfig", func(t *testing.T) { + h := NewRiskHandler(nil, nil) + assert.NotNil(t, h) + assert.NotNil(t, h.cfg, "cfg should be non-nil even when nil is passed") + assert.Equal(t, &config.RiskConfig{}, h.cfg) + assert.NotNil(t, h.riskService) + }) +} + +// TestNewTradesHandler verifies that NewTradesHandler returns a non-nil handler. +func TestNewTradesHandler(t *testing.T) { + h := NewTradesHandler(nil) + assert.NotNil(t, h) + assert.Nil(t, h.db) +} + +// TestNewPerformanceHandler verifies that NewPerformanceHandler returns a non-nil handler. +func TestNewPerformanceHandler(t *testing.T) { + h := NewPerformanceHandler(nil) + assert.NotNil(t, h) + assert.Nil(t, h.db) +} + +func TestBuildBreaker(t *testing.T) { + tests := []struct { + name string + current float64 + threshold float64 + expectedStatus string + }{ + {"disabled when threshold zero", 100.0, 0.0, "DISABLED"}, + {"disabled when threshold negative", 50.0, -1.0, "DISABLED"}, + {"triggered when current equals threshold", 100.0, 100.0, "TRIGGERED"}, + {"triggered when current exceeds threshold", 110.0, 100.0, "TRIGGERED"}, + {"warning when at 80 percent", 80.0, 100.0, "WARNING"}, + {"warning just above 80 percent", 81.0, 100.0, "WARNING"}, + {"ok when below 80 percent", 79.0, 100.0, "OK"}, + {"ok at zero current", 0.0, 100.0, "OK"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := buildBreaker("TestBreaker", tc.current, tc.threshold) + assert.Equal(t, tc.expectedStatus, result["status"]) + assert.Equal(t, "TestBreaker", result["name"]) + assert.Equal(t, tc.threshold, result["threshold"]) + // current is rounded to 2dp + assert.Equal(t, math.Round(tc.current*100)/100, result["current"]) + }) + } +} diff --git a/internal/backtest/job_unit_test.go b/internal/backtest/job_unit_test.go new file mode 100644 index 0000000..4e7c97c --- /dev/null +++ b/internal/backtest/job_unit_test.go @@ -0,0 +1,83 @@ +package backtest + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestValidateJob tests the validateJob method without a DB connection. +func TestValidateJob(t *testing.T) { + m := &JobManager{} // nil db is fine — validateJob doesn't touch the DB + + validJob := func() *BacktestJob { + return &BacktestJob{ + Name: "test", + StartDate: time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC), + EndDate: time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC), + Symbols: []string{"BTCUSDT"}, + InitialCapital: 10000.0, + StrategyConfig: map[string]interface{}{"type": "trend"}, + } + } + + t.Run("valid job passes", func(t *testing.T) { + require.NoError(t, m.validateJob(validJob())) + }) + + t.Run("empty name", func(t *testing.T) { + j := validJob() + j.Name = "" + assert.Error(t, m.validateJob(j)) + }) + + t.Run("end before start", func(t *testing.T) { + j := validJob() + j.EndDate = j.StartDate.Add(-time.Hour) + assert.Error(t, m.validateJob(j)) + }) + + t.Run("equal start and end", func(t *testing.T) { + j := validJob() + j.EndDate = j.StartDate + assert.Error(t, m.validateJob(j)) + }) + + t.Run("no symbols", func(t *testing.T) { + j := validJob() + j.Symbols = nil + assert.Error(t, m.validateJob(j)) + }) + + t.Run("zero capital", func(t *testing.T) { + j := validJob() + j.InitialCapital = 0 + assert.Error(t, m.validateJob(j)) + }) + + t.Run("negative capital", func(t *testing.T) { + j := validJob() + j.InitialCapital = -1 + assert.Error(t, m.validateJob(j)) + }) + + t.Run("empty strategy config", func(t *testing.T) { + j := validJob() + j.StrategyConfig = nil + assert.Error(t, m.validateJob(j)) + }) +} + +func TestGetValue(t *testing.T) { + v := 3.14 + assert.Equal(t, 3.14, getValue(&v)) + assert.Equal(t, 0.0, getValue(nil)) +} + +func TestGetIntValue(t *testing.T) { + n := 42 + assert.Equal(t, 42, getIntValue(&n)) + assert.Equal(t, 0, getIntValue(nil)) +} diff --git a/internal/db/utils_test.go b/internal/db/utils_test.go new file mode 100644 index 0000000..5f961d2 --- /dev/null +++ b/internal/db/utils_test.go @@ -0,0 +1,22 @@ +package db + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPtrFloat64(t *testing.T) { + v := PtrFloat64(3.14) + require.NotNil(t, v) + assert.Equal(t, 3.14, *v) + + zero := PtrFloat64(0.0) + require.NotNil(t, zero) + assert.Equal(t, 0.0, *zero) + + neg := PtrFloat64(-99.5) + require.NotNil(t, neg) + assert.Equal(t, -99.5, *neg) +} diff --git a/internal/exchange/safety_guard_nil_test.go b/internal/exchange/safety_guard_nil_test.go new file mode 100644 index 0000000..48ca30a --- /dev/null +++ b/internal/exchange/safety_guard_nil_test.go @@ -0,0 +1,117 @@ +package exchange + +// Tests for service safety-guard methods when the guard is initialized but disabled +// (i.e., NewServicePaper — SafetyGuardConfig{Enabled: false}). +// +// Note: safetyGuard is always non-nil; the nil branch in each method is dead code +// because NewService always calls risk.NewSafetyGuard(config.SafetyGuard). + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newPaperServiceNoSafety(t *testing.T) *Service { + t.Helper() + s := NewServicePaper(nil) // nil DB, no safety guard config → guard initialized but disabled + return s +} + +func TestGetSafetyGuardStats_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.GetSafetyGuardStats(context.Background(), nil) + require.NoError(t, err) + m := result.(map[string]interface{}) + // Guard is non-nil, so enabled is always true in the non-nil branch + assert.Equal(t, true, m["enabled"]) + assert.Equal(t, float64(0), m["daily_pnl"]) + assert.Equal(t, int(0), m["daily_trade_count"]) +} + +func TestSetSafetyGuardCapital_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.SetSafetyGuardCapital(context.Background(), map[string]interface{}{"capital": 1000.0}) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, true, m["success"]) + assert.Equal(t, 1000.0, m["capital"]) +} + +func TestRecordTradePnL_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.RecordTradePnL(context.Background(), map[string]interface{}{"pnl": 50.0}) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, true, m["success"]) + assert.Equal(t, 50.0, m["pnl_recorded"]) +} + +func TestResetSafetyGuardCircuitBreaker_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.ResetSafetyGuardCircuitBreaker(context.Background(), nil) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, true, m["success"]) +} + +func TestResetDailyCounters_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + // nil args → capital extraction fails: "capital is required" + _, err := s.ResetDailyCounters(context.Background(), nil) + assert.Error(t, err) + + // valid call succeeds + result, err := s.ResetDailyCounters(context.Background(), map[string]interface{}{"capital": 5000.0}) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, true, m["success"]) + assert.Equal(t, 5000.0, m["new_capital"]) +} + +func TestEmergencyStop_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.EmergencyStop(context.Background(), map[string]interface{}{"reason": "test"}) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, true, m["success"]) + assert.Equal(t, "test", m["reason"]) +} + +func TestClearEmergencyStop_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.ClearEmergencyStop(context.Background(), nil) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, true, m["success"]) + assert.Equal(t, false, m["was_active"]) +} + +func TestGetEmergencyStopStatus_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.GetEmergencyStopStatus(context.Background(), nil) + require.NoError(t, err) + m := result.(map[string]interface{}) + assert.Equal(t, false, m["active"]) +} + +func TestGetTradingHoursStatus_NoGuard(t *testing.T) { + s := newPaperServiceNoSafety(t) + result, err := s.GetTradingHoursStatus(context.Background(), nil) + require.NoError(t, err) + m := result.(map[string]interface{}) + // TradingHours.Enabled is false in zero config + assert.Equal(t, false, m["enabled"]) + // IsWithinTradingHours returns true when disabled + assert.Equal(t, true, m["within_hours"]) +} + +func TestSetMarketPrice_Service(t *testing.T) { + s := newPaperServiceNoSafety(t) + // SetMarketPrice delegates to the mock exchange — should not panic + s.SetMarketPrice("BTCUSDT", 45000.0) + price := s.exchange.GetMarketPrice("BTCUSDT") + assert.Equal(t, 45000.0, price) +} diff --git a/internal/market/pure_unit_test.go b/internal/market/pure_unit_test.go new file mode 100644 index 0000000..a533b0f --- /dev/null +++ b/internal/market/pure_unit_test.go @@ -0,0 +1,90 @@ +package market + +import ( + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCoinGeckoIDToBinanceSymbol(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"bitcoin", "BTCUSDT"}, + {"ethereum", "ETHUSDT"}, + {"solana", "SOLUSDT"}, + {"cardano", "ADAUSDT"}, + {"ripple", "XRPUSDT"}, + {"dogecoin", "DOGEUSDT"}, + {"polkadot", "DOTUSDT"}, + {"avalanche", "AVAXUSDT"}, + {"chainlink", "LINKUSDT"}, + {"polygon", "MATICUSDT"}, + // unknown → uppercase + USDT + {"shiba", "SHIBAUSDT"}, + {"unknowncoin", "UNKNOWNCOINUSDT"}, + } + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + assert.Equal(t, tc.want, CoinGeckoIDToBinanceSymbol(tc.input)) + }) + } +} + +func TestEstimateOHLCV(t *testing.T) { + t.Run("nil / empty input", func(t *testing.T) { + result := EstimateOHLCV(nil) + require.NotNil(t, result) + assert.Empty(t, result.Close) + }) + + t.Run("single price uses min range", func(t *testing.T) { + result := EstimateOHLCV([]float64{100.0}) + require.Len(t, result.High, 1) + assert.Greater(t, result.High[0], 100.0, "high should be above close") + assert.Less(t, result.Low[0], 100.0, "low should be below close") + assert.Equal(t, 100.0, result.Close[0]) + }) + + t.Run("two prices", func(t *testing.T) { + closes := []float64{100.0, 110.0} + result := EstimateOHLCV(closes) + require.Len(t, result.High, 2) + for i := range closes { + assert.GreaterOrEqual(t, result.High[i], closes[i]) + assert.LessOrEqual(t, result.Low[i], closes[i]) + assert.Equal(t, closes[i], result.Close[i]) + } + }) + + t.Run("flat prices use min range of 0.1 pct", func(t *testing.T) { + closes := []float64{1000.0, 1000.0, 1000.0} + result := EstimateOHLCV(closes) + // halfRange = max(0, 0) → falls back to minRange = 1.0 → halfRange/2 = 0.5 + expectedHalfRange := 1000.0 * 0.001 / 2.0 + assert.InDelta(t, 1000.0+expectedHalfRange, result.High[1], 1e-9) + assert.InDelta(t, 1000.0-expectedHalfRange, result.Low[1], 1e-9) + }) + + t.Run("high is always >= close and low <= close", func(t *testing.T) { + closes := []float64{50000, 51000, 49000, 52000, 48000} + result := EstimateOHLCV(closes) + for i, c := range closes { + assert.GreaterOrEqual(t, result.High[i], c) + assert.LessOrEqual(t, result.Low[i], c) + assert.False(t, math.IsNaN(result.High[i])) + assert.False(t, math.IsNaN(result.Low[i])) + } + }) +} + +func TestParseStringFloat(t *testing.T) { + assert.Equal(t, 3.14, ParseStringFloat(3.14)) + assert.Equal(t, 42.0, ParseStringFloat("42")) + assert.Equal(t, 0.0, ParseStringFloat("notanumber")) + assert.Equal(t, 0.0, ParseStringFloat(nil)) + assert.Equal(t, 0.0, ParseStringFloat(42)) // int is not handled +} diff --git a/internal/models/convert_test.go b/internal/models/convert_test.go new file mode 100644 index 0000000..68d6c0f --- /dev/null +++ b/internal/models/convert_test.go @@ -0,0 +1,243 @@ +package models + +import ( + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ajitpratap0/cryptofunk/internal/db" +) + +// TestBinancePositionToUnified_OpenPosition tests conversion of an open Binance position +// with nil optional fields. +func TestBinancePositionToUnified_OpenPosition(t *testing.T) { + posID := uuid.New() + entryTime := time.Now().Add(-1 * time.Hour) + + p := &db.Position{ + ID: posID, + Symbol: "BTC/USDT", + Side: db.PositionSideLong, + EntryPrice: 50000.0, + Quantity: 0.5, + EntryTime: entryTime, + // nil optionals: SessionID, UnrealizedPnL, RealizedPnL, ExitTime, ExitPrice, EntryReason + } + + u := BinancePositionToUnified(p) + + assert.Equal(t, posID, u.ID) + assert.Equal(t, PlatformBinance, u.Platform) + assert.Equal(t, "BTC/USDT", u.Symbol) + assert.Equal(t, "long", u.Side, "Side should be lowercased") + assert.InDelta(t, 50000.0, u.EntryPrice, 1e-9) + assert.InDelta(t, 0.5, u.Quantity, 1e-9) + assert.InDelta(t, 50000.0*0.5, u.CostBasis, 1e-9, "CostBasis = EntryPrice * Quantity") + assert.Equal(t, PositionStatusOpen, u.Status) + assert.Nil(t, u.ClosedAt) + assert.InDelta(t, 0.0, u.UnrealizedPnL, 1e-9, "nil UnrealizedPnL should default to 0") + assert.InDelta(t, 0.0, u.RealizedPnL, 1e-9, "nil RealizedPnL should default to 0") + assert.Equal(t, uuid.Nil, u.SessionID, "nil SessionID should default to uuid.Nil") +} + +// TestBinancePositionToUnified_ClosedPosition tests conversion of a closed position with all +// optional fields populated. +func TestBinancePositionToUnified_ClosedPosition(t *testing.T) { + posID := uuid.New() + sessionID := uuid.New() + entryTime := time.Now().Add(-2 * time.Hour) + exitTime := time.Now().Add(-1 * time.Hour) + exitPrice := 52000.0 + unrealizedPnL := 0.0 + realizedPnL := 1000.0 + entryReason := "technical-agent" + + p := &db.Position{ + ID: posID, + SessionID: &sessionID, + Symbol: "ETH/USDT", + Side: db.PositionSideShort, + EntryPrice: 3000.0, + Quantity: 2.0, + EntryTime: entryTime, + ExitTime: &exitTime, + ExitPrice: &exitPrice, + UnrealizedPnL: &unrealizedPnL, + RealizedPnL: &realizedPnL, + EntryReason: &entryReason, + } + + u := BinancePositionToUnified(p) + + assert.Equal(t, posID, u.ID) + assert.Equal(t, PlatformBinance, u.Platform) + assert.Equal(t, "short", u.Side, "Side should be lowercased") + assert.Equal(t, PositionStatusClosed, u.Status) + assert.NotNil(t, u.ClosedAt) + assert.Equal(t, exitTime, *u.ClosedAt) + assert.InDelta(t, 52000.0, u.CurrentPrice, 1e-9) + assert.InDelta(t, unrealizedPnL, u.UnrealizedPnL, 1e-9) + assert.InDelta(t, realizedPnL, u.RealizedPnL, 1e-9) + assert.Equal(t, sessionID, u.SessionID) + assert.Equal(t, "technical-agent", u.Agent) + assert.InDelta(t, 3000.0*2.0, u.CostBasis, 1e-9) +} + +// TestBinanceTradeToUnified tests conversion of a Binance trade+order pair. +func TestBinanceTradeToUnified(t *testing.T) { + tradeID := uuid.New() + orderID := uuid.New() + positionID := uuid.New() + executedAt := time.Now() + + trade := &db.Trade{ + ID: tradeID, + OrderID: orderID, + Symbol: "BTC/USDT", + Side: db.OrderSideBuy, + Price: 48000.0, + Quantity: 0.1, + QuoteQuantity: 4800.0, + ExecutedAt: executedAt, + } + + order := &db.Order{ + ID: orderID, + PositionID: &positionID, + } + + u := BinanceTradeToUnified(trade, order) + + assert.Equal(t, tradeID, u.ID) + assert.Equal(t, PlatformBinance, u.Platform) + assert.Equal(t, "BTC/USDT", u.Symbol) + assert.InDelta(t, 48000.0, u.Price, 1e-9) + assert.InDelta(t, 0.1, u.Quantity, 1e-9) + assert.InDelta(t, 4800.0, u.Amount, 1e-9) + assert.Equal(t, positionID, u.PositionID) + assert.Equal(t, "BUY", u.Action) + assert.Equal(t, executedAt, u.Timestamp) +} + +// TestBinanceTradeToUnified_NilOrder verifies that a nil order doesn't panic and PositionID +// is left as zero value. +func TestBinanceTradeToUnified_NilOrder(t *testing.T) { + trade := &db.Trade{ + ID: uuid.New(), + Symbol: "SOL/USDT", + Side: db.OrderSideSell, + Price: 100.0, + } + + u := BinanceTradeToUnified(trade, nil) + + assert.Equal(t, PlatformBinance, u.Platform) + assert.Equal(t, "SOL/USDT", u.Symbol) + assert.Equal(t, uuid.Nil, u.PositionID, "PositionID should be zero when order is nil") + assert.Equal(t, "SELL", u.Action) +} + +// TestNewEmptyPortfolio verifies the portfolio is non-nil with an empty positions slice. +func TestNewEmptyPortfolio(t *testing.T) { + p := NewEmptyPortfolio() + + require.NotNil(t, p) + require.NotNil(t, p.Positions) + assert.Empty(t, p.Positions) + require.NotNil(t, p.ByPlatform) + assert.Contains(t, p.ByPlatform, PlatformBinance) + assert.Contains(t, p.ByPlatform, PlatformPolymarket) + assert.Equal(t, 0, p.OpenPositions) + assert.InDelta(t, 0.0, p.TotalValue, 1e-9) +} + +// TestUnifiedPortfolio_AddPosition verifies that adding a position updates the portfolio. +func TestUnifiedPortfolio_AddPosition(t *testing.T) { + p := NewEmptyPortfolio() + + pos := UnifiedPosition{ + ID: uuid.New(), + Platform: PlatformBinance, + Symbol: "BTC/USDT", + EntryPrice: 50000.0, + CurrentPrice: 51000.0, + Quantity: 1.0, + UnrealizedPnL: 1000.0, + RealizedPnL: 500.0, + Status: PositionStatusOpen, + } + + p.AddPosition(pos) + + require.Len(t, p.Positions, 1) + assert.Equal(t, pos, p.Positions[0]) + assert.Equal(t, 1, p.OpenPositions) + assert.InDelta(t, 1000.0, p.UnrealizedPnL, 1e-9) + assert.InDelta(t, 500.0, p.RealizedPnL, 1e-9) + assert.InDelta(t, 1500.0, p.TotalPnL, 1e-9) + assert.InDelta(t, 51000.0, p.TotalValue, 1e-9, "TotalValue = CurrentPrice * Quantity") + + binanceSummary := p.ByPlatform[PlatformBinance] + assert.Equal(t, 1, binanceSummary.PositionCount) + assert.InDelta(t, 51000.0, binanceSummary.TotalValue, 1e-9) +} + +// TestUnifiedPortfolio_AddPosition_Closed verifies that adding a closed position does not +// increment OpenPositions or UnrealizedPnL. +func TestUnifiedPortfolio_AddPosition_Closed(t *testing.T) { + p := NewEmptyPortfolio() + + closedAt := time.Now() + pos := UnifiedPosition{ + ID: uuid.New(), + Platform: PlatformBinance, + Symbol: "ETH/USDT", + CurrentPrice: 3000.0, + Quantity: 1.0, + RealizedPnL: 200.0, + Status: PositionStatusClosed, + ClosedAt: &closedAt, + } + + p.AddPosition(pos) + + assert.Equal(t, 0, p.OpenPositions) + assert.InDelta(t, 0.0, p.UnrealizedPnL, 1e-9) + assert.InDelta(t, 200.0, p.RealizedPnL, 1e-9) +} + +// TestUnifiedPortfolio_SetPlatformTradeCount verifies that trade counts are stored and +// TotalTrades is updated correctly. +func TestUnifiedPortfolio_SetPlatformTradeCount(t *testing.T) { + p := NewEmptyPortfolio() + + p.SetPlatformTradeCount(PlatformBinance, 10) + p.SetPlatformTradeCount(PlatformPolymarket, 5) + + assert.Equal(t, 10, p.ByPlatform[PlatformBinance].TradeCount) + assert.Equal(t, 5, p.ByPlatform[PlatformPolymarket].TradeCount) + assert.Equal(t, 15, p.TotalTrades) + + // Updating one platform recalculates total correctly. + p.SetPlatformTradeCount(PlatformBinance, 20) + assert.Equal(t, 20, p.ByPlatform[PlatformBinance].TradeCount) + assert.Equal(t, 25, p.TotalTrades) +} + +// TestUnifiedPosition_IsOpen tests the IsOpen method for both open and closed positions. +func TestUnifiedPosition_IsOpen(t *testing.T) { + open := &UnifiedPosition{ + Status: PositionStatusOpen, + } + assert.True(t, open.IsOpen(), "position with status OPEN should return true from IsOpen") + + closedAt := time.Now() + closed := &UnifiedPosition{ + Status: PositionStatusClosed, + ClosedAt: &closedAt, + } + assert.False(t, closed.IsOpen(), "position with status CLOSED should return false from IsOpen") +} diff --git a/internal/polymarket/analyzer/analyzer_unit_test.go b/internal/polymarket/analyzer/analyzer_unit_test.go new file mode 100644 index 0000000..a1b9720 --- /dev/null +++ b/internal/polymarket/analyzer/analyzer_unit_test.go @@ -0,0 +1,78 @@ +package analyzer + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ajitpratap0/cryptofunk/internal/polymarket/gamma" +) + +func TestParseResponse(t *testing.T) { + t.Run("valid JSON response", func(t *testing.T) { + resp := `{"predicted_probability":0.75,"confidence":0.8,"reasoning":"strong evidence","action":"BUY YES"}` + market := &gamma.Market{ + ConditionID: "123", + Question: "Test?", + OutcomeYesPrice: 0.5, + OutcomeNoPrice: 0.5, + } + + analysis := parseResponse(resp, market) + + require.NotNil(t, analysis) + assert.InDelta(t, 0.75, analysis.PredictedProb, 1e-9) + assert.InDelta(t, 0.8, analysis.Confidence, 1e-9) + assert.Equal(t, "BUY YES", analysis.Action) + assert.InDelta(t, 0.25, analysis.Edge, 1e-9) // 0.75 - 0.5 + assert.Equal(t, "123", analysis.MarketID) + }) + + t.Run("JSON embedded in surrounding text", func(t *testing.T) { + resp := `Some preamble text {"predicted_probability":0.3,"confidence":0.6,"reasoning":"weak","action":"BUY NO"} trailing text` + market := &gamma.Market{ + ConditionID: "456", + Question: "Will X happen?", + OutcomeYesPrice: 0.5, + OutcomeNoPrice: 0.5, + } + + analysis := parseResponse(resp, market) + + require.NotNil(t, analysis) + assert.Equal(t, "BUY NO", analysis.Action) + assert.InDelta(t, 0.3, analysis.PredictedProb, 1e-9) + }) + + t.Run("invalid response with no valid JSON", func(t *testing.T) { + resp := "I cannot analyze this market" + market := &gamma.Market{ + ConditionID: "789", + Question: "Another question?", + OutcomeYesPrice: 0.6, + OutcomeNoPrice: 0.4, + } + + analysis := parseResponse(resp, market) + + require.NotNil(t, analysis) + assert.Equal(t, "SKIP", analysis.Action) + assert.InDelta(t, 0.0, analysis.PredictedProb, 1e-9) + }) + + t.Run("empty string response", func(t *testing.T) { + resp := "" + market := &gamma.Market{ + ConditionID: "000", + Question: "Empty test?", + OutcomeYesPrice: 0.5, + OutcomeNoPrice: 0.5, + } + + analysis := parseResponse(resp, market) + + require.NotNil(t, analysis) + assert.Equal(t, "SKIP", analysis.Action) + }) +} diff --git a/internal/polymarket/news/news_unit_test.go b/internal/polymarket/news/news_unit_test.go new file mode 100644 index 0000000..d6ed120 --- /dev/null +++ b/internal/polymarket/news/news_unit_test.go @@ -0,0 +1,30 @@ +package news + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStripHTML(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {"plain text unchanged", "hello world", "hello world"}, + {"strips simple tag", "bold", "bold"}, + {"strips multiple tags", "

hello world

", "hello world"}, + {"empty string", "", ""}, + {"only tags", "
", ""}, + {"nested tags", "
text
", "text"}, + {"tag with attributes", `link`, "link"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := stripHTML(tc.input) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/polymarket/resolver/resolver_unit_test.go b/internal/polymarket/resolver/resolver_unit_test.go new file mode 100644 index 0000000..5deeb89 --- /dev/null +++ b/internal/polymarket/resolver/resolver_unit_test.go @@ -0,0 +1,60 @@ +package resolver + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCalculatePredictionPnl(t *testing.T) { + tests := []struct { + name string + predictedProb float64 + marketPrice float64 + outcome float64 + expected float64 + }{ + { + name: "predicted > market, outcome YES (win)", + predictedProb: 0.7, + marketPrice: 0.5, + outcome: 1.0, + expected: 0.5, // outcome - marketPrice = 1.0 - 0.5 + }, + { + name: "predicted > market, outcome NO (loss)", + predictedProb: 0.7, + marketPrice: 0.5, + outcome: 0.0, + expected: -0.5, // outcome - marketPrice = 0.0 - 0.5 + }, + { + name: "predicted < market, outcome NO (win)", + predictedProb: 0.3, + marketPrice: 0.5, + outcome: 0.0, + expected: 0.5, // (1-outcome) - (1-marketPrice) = 1.0 - 0.5 + }, + { + name: "predicted < market, outcome YES (loss)", + predictedProb: 0.3, + marketPrice: 0.5, + outcome: 1.0, + expected: -0.5, // (1-outcome) - (1-marketPrice) = 0.0 - 0.5 + }, + { + name: "predicted == market, goes to NO branch, outcome NO (win)", + predictedProb: 0.5, + marketPrice: 0.5, + outcome: 0.0, + expected: 0.5, // (1-0.0) - (1-0.5) = 1.0 - 0.5 + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := calculatePredictionPnl(tc.predictedProb, tc.marketPrice, tc.outcome) + assert.InDelta(t, tc.expected, got, 1e-9) + }) + } +}