From 7fad8067e1358335c6b45c61a05ab7ce3e011686 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Wed, 22 Jul 2026 17:36:29 +0700 Subject: [PATCH] fix(portfolio): tighten mobile column formatting --- internal/modules/coin/format.go | 19 +++++---- internal/modules/coin/format_test.go | 54 +++++++++++++------------- internal/modules/coin/handlers_test.go | 13 ++++--- internal/modules/coin/views.go | 11 +++--- internal/modules/stock/format.go | 23 ++++++++--- internal/modules/stock/format_test.go | 26 +++++++++++-- internal/modules/stock/handlers.go | 11 +++--- internal/modules/stock/stats_test.go | 27 ++++++------- 8 files changed, 114 insertions(+), 70 deletions(-) diff --git a/internal/modules/coin/format.go b/internal/modules/coin/format.go index 8817849..4a0c689 100644 --- a/internal/modules/coin/format.go +++ b/internal/modules/coin/format.go @@ -25,15 +25,15 @@ func FormatUSD(n float64) string { } // formatCompactUSD renders a position-table amount with at most three scaled -// fractional digits while preserving the module's dollar/sign convention. +// fractional digits. The coin portfolio makes USD implicit and omits "$". func formatCompactUSD(n float64) string { if math.IsNaN(n) || math.IsInf(n, 0) { return "invalid USD" } // Use the full formatter's cent rounding at the base/k boundary so an - // amount displayed as $1,000.00 is promoted to $1k instead. + // amount displayed as 1,000.00 is promoted to 1k instead. if math.Round(math.Abs(n)*100)/100 < 1_000 { - return FormatUSD(n) + return strings.Replace(FormatUSD(n), "$", "", 1) } sign := "" @@ -56,7 +56,7 @@ func formatCompactUSD(n float64) string { scaled = math.Round(n/divisor*1_000) / 1_000 } amount := strings.TrimRight(strings.TrimRight(strconv.FormatFloat(scaled, 'f', 3, 64), "0"), ".") - return sign + "$" + amount + suffixes[suffixIndex] + return sign + amount + suffixes[suffixIndex] } func FormatCoinQty(n float64) string { @@ -73,11 +73,16 @@ func FormatPnLUSD(currentValue, invested float64) string { return formatPnLUSD(currentValue, invested, FormatUSD) } -func formatPortfolioPositionPnLUSD(currentValue, invested float64) string { - return formatPnLUSD(currentValue, invested, formatCompactUSD) +func formatPortfolioPositionPnLUSD(currentValue, invested float64) (string, string) { + return formatPnLUSDParts(currentValue, invested, formatCompactUSD) } func formatPnLUSD(currentValue, invested float64, formatAmount func(float64) string) string { + amount, percentage := formatPnLUSDParts(currentValue, invested, formatAmount) + return amount + " (" + percentage + ")" +} + +func formatPnLUSDParts(currentValue, invested float64, formatAmount func(float64) string) (string, string) { diff := currentValue - invested pct := 0.0 if invested > 0 { @@ -87,7 +92,7 @@ func formatPnLUSD(currentValue, invested float64, formatAmount func(float64) str if diff >= 0 { sign = "+" } - return sign + formatAmount(diff) + " (" + sign + strconv.FormatFloat(pct, 'f', 2, 64) + "%)" + return sign + formatAmount(diff), sign + strconv.FormatFloat(pct, 'f', 2, 64) + "%" } func groupDigits(s string) string { diff --git a/internal/modules/coin/format_test.go b/internal/modules/coin/format_test.go index 140ff57..ed4408d 100644 --- a/internal/modules/coin/format_test.go +++ b/internal/modules/coin/format_test.go @@ -10,26 +10,26 @@ func TestFormatCompactUSD(t *testing.T) { in float64 want string }{ - {0, "$0.00"}, - {999.99, "$999.99"}, - {999.994, "$999.99"}, - {999.999, "$1k"}, - {1_000, "$1k"}, - {25_350, "$25.35k"}, - {25_351, "$25.351k"}, - {999_499, "$999.499k"}, - {999_999.4, "$999.999k"}, - {999_999.5, "$1M"}, - {1_000_000, "$1M"}, - {1_234_000, "$1.234M"}, - {126_000_000, "$126M"}, - {999_999_999.5, "$1B"}, - {1_250_000_000, "$1.25B"}, - {999_999_999_999.5, "$1T"}, - {1_000_000_000_000, "$1T"}, - {-25_350, "-$25.35k"}, - {-999.999, "-$1k"}, - {-1_250_000_000, "-$1.25B"}, + {0, "0.00"}, + {999.99, "999.99"}, + {999.994, "999.99"}, + {999.999, "1k"}, + {1_000, "1k"}, + {25_350, "25.35k"}, + {25_351, "25.351k"}, + {999_499, "999.499k"}, + {999_999.4, "999.999k"}, + {999_999.5, "1M"}, + {1_000_000, "1M"}, + {1_234_000, "1.234M"}, + {126_000_000, "126M"}, + {999_999_999.5, "1B"}, + {1_250_000_000, "1.25B"}, + {999_999_999_999.5, "1T"}, + {1_000_000_000_000, "1T"}, + {-25_350, "-25.35k"}, + {-999.999, "-1k"}, + {-1_250_000_000, "-1.25B"}, {math.NaN(), "invalid USD"}, {math.Inf(1), "invalid USD"}, } @@ -40,18 +40,20 @@ func TestFormatCompactUSD(t *testing.T) { } } -func TestFormatPortfolioPositionPnLUSDUsesCompactAmountAndFullPercentage(t *testing.T) { +func TestFormatPortfolioPositionPnLUSDSplitsAmountAndPercentage(t *testing.T) { tests := []struct { current float64 invested float64 - want string + amount string + percent string }{ - {1_250_000, 1_000_000, "+$250k (+25.00%)"}, - {750_000, 1_000_000, "-$250k (-25.00%)"}, + {1_250_000, 1_000_000, "+250k", "+25.00%"}, + {750_000, 1_000_000, "-250k", "-25.00%"}, } for _, test := range tests { - if got := formatPortfolioPositionPnLUSD(test.current, test.invested); got != test.want { - t.Errorf("formatPortfolioPositionPnLUSD(%v, %v): got %q, want %q", test.current, test.invested, got, test.want) + amount, percent := formatPortfolioPositionPnLUSD(test.current, test.invested) + if amount != test.amount || percent != test.percent { + t.Errorf("formatPortfolioPositionPnLUSD(%v, %v): got (%q, %q), want (%q, %q)", test.current, test.invested, amount, percent, test.amount, test.percent) } } } diff --git a/internal/modules/coin/handlers_test.go b/internal/modules/coin/handlers_test.go index efad471..0928784 100644 --- a/internal/modules/coin/handlers_test.go +++ b/internal/modules/coin/handlers_test.go @@ -334,7 +334,7 @@ func TestStatsWithAndWithoutPrice(t *testing.T) { t.Fatalf("handleStats: %v", err) } text := rb.LastSent().Text() - for _, want := range []string{"Coin Portfolio", "
", "BTC", "0.01", "$50k", "$500.00", "+$0.00 (+0.00%)", "P&L"} {
+	for _, want := range []string{"Coin Portfolio", "
", "Sym", "BTC", "0.01", "50k", "500.00", "+0.00", "+0.00%", "P&L"} {
 		if !strings.Contains(text, want) {
 			t.Fatalf("stats missing %q in %q", want, text)
 		}
@@ -345,7 +345,7 @@ func TestStatsWithAndWithoutPrice(t *testing.T) {
 		t.Fatalf("handleStats no price: %v", err)
 	}
 	rb.AssertSentText(t, "N/A")
-	rb.AssertSentText(t, "$50k")
+	rb.AssertSentText(t, "50k")
 	if strings.Contains(rb.LastSent().Text(), "Account P&L: +") || strings.Contains(rb.LastSent().Text(), "Account P&L: -") {
 		t.Fatalf("partial prices must not show numeric account P&L: %q", rb.LastSent().Text())
 	}
@@ -369,10 +369,11 @@ func TestStatsCompactsOnlyPositionMonetaryCells(t *testing.T) {
 	text := rb.LastSent().Text()
 	for _, want := range []string{
 		"BTC",
-		"$1k",
-		"$1.25k",
-		"$2.5M",
-		"+$500k (+25.00%)",
+		"1k",
+		"1.25k",
+		"2.5M",
+		"+500k",
+		"+25.00%",
 		"$1,234.00",
 		"$2,501,234.00",
 		"+$500,000.00 (+25.00%)",
diff --git a/internal/modules/coin/views.go b/internal/modules/coin/views.go
index 3880671..5ad8a0f 100644
--- a/internal/modules/coin/views.go
+++ b/internal/modules/coin/views.go
@@ -43,20 +43,21 @@ func (s *state) handleStats(ctx context.Context, b *bot.Bot, update *models.Upda
 				value := held * price.USD
 				if !isPositiveFinite(value) || !isPositiveFinite(average) {
 					missingPrice = true
-					positions = append(positions, []string{symbol, FormatCoinQty(held), "N/A", "N/A", "N/A", "N/A"})
+					positions = append(positions, []string{symbol, FormatCoinQty(held), "N/A", "N/A", "N/A", "N/A", "N/A"})
 					continue
 				}
 				totalValue += value
 				totalBasis += basis
-				positions = append(positions, []string{symbol, FormatCoinQty(held), formatCompactUSD(average), formatCompactUSD(price.USD), formatCompactUSD(value), formatPortfolioPositionPnLUSD(value, basis)})
+				pnlAmount, pnlPercentage := formatPortfolioPositionPnLUSD(value, basis)
+				positions = append(positions, []string{symbol, FormatCoinQty(held), formatCompactUSD(average), formatCompactUSD(price.USD), formatCompactUSD(value), pnlAmount, pnlPercentage})
 			} else {
 				log.Error("coin_fetch_price", "symbol", symbol, "err", err)
 				missingPrice = true
-				positions = append(positions, []string{symbol, FormatCoinQty(held), formatCompactUSD(average), "N/A", "N/A", "N/A"})
+				positions = append(positions, []string{symbol, FormatCoinQty(held), formatCompactUSD(average), "N/A", "N/A", "N/A", "N/A"})
 			}
 		} else {
 			missingPrice = true
-			positions = append(positions, []string{symbol, FormatCoinQty(held), formatCompactUSD(average), "N/A", "N/A", "N/A"})
+			positions = append(positions, []string{symbol, FormatCoinQty(held), formatCompactUSD(average), "N/A", "N/A", "N/A", "N/A"})
 		}
 	}
 	var summary [][]string
@@ -101,7 +102,7 @@ func portfolioTableReply(title string, positions, summary [][]string) string {
 			rows = append(rows, []string{"… " + strconv.Itoa(omitted) + " omitted"})
 		}
 		reply := "" + title + "\n" +
-			chathelper.MonospaceTable([]string{"Ticker", "Qty", "Avg", "Now", "Value", "Unrealized P&L"}, rows) + "\n" +
+			chathelper.MonospaceTable([]string{"Sym", "Qty", "Avg", "Now", "Val", "P&L", "%"}, rows) + "\n" +
 			chathelper.MonospaceTable([]string{"Metric", "Value"}, summary)
 		if len(reply) <= portfolioReplyLimit || len(positions) == 0 {
 			return reply
diff --git a/internal/modules/stock/format.go b/internal/modules/stock/format.go
index 37d7efe..9d8741a 100644
--- a/internal/modules/stock/format.go
+++ b/internal/modules/stock/format.go
@@ -15,8 +15,8 @@ func FormatVND(n float64) string {
 	return formatVNDNumber(n) + " VND"
 }
 
-// formatVNDNumber renders a VND amount without its currency suffix. Portfolio
-// tables use this because their title declares the currency once.
+// formatVNDNumber renders a VND amount without its currency suffix. Stock
+// portfolio summaries use VND as their implicit currency.
 func formatVNDNumber(n float64) string {
 	return formatGroupedInteger(int64(math.Round(n)))
 }
@@ -54,6 +54,14 @@ func formatCompactVND(n float64) string {
 	return result + suffixes[suffixIndex]
 }
 
+// formatThousandVND renders a VND amount in thousands without a currency
+// suffix. Stock portfolio Avg and Now columns declare this unit by convention.
+func formatThousandVND(n float64) string {
+	scaled := math.Round(n) / 1_000
+	result := strings.TrimRight(strings.TrimRight(strconv.FormatFloat(scaled, 'f', 3, 64), "0"), ".")
+	return strings.Replace(result, ".", ",", 1)
+}
+
 func formatGroupedInteger(n int64) string {
 	abs := strconv.FormatInt(absInt64(n), 10)
 	var sb strings.Builder
@@ -108,11 +116,16 @@ func formatPortfolioPnL(currentValue, invested float64) string {
 	return formatPnL(currentValue, invested, formatVNDNumber)
 }
 
-func formatPortfolioPositionPnL(currentValue, invested float64) string {
-	return formatPnL(currentValue, invested, formatCompactVND)
+func formatPortfolioPositionPnL(currentValue, invested float64) (string, string) {
+	return formatPnLParts(currentValue, invested, formatCompactVND)
 }
 
 func formatPnL(currentValue, invested float64, formatAmount func(float64) string) string {
+	amount, percentage := formatPnLParts(currentValue, invested, formatAmount)
+	return amount + " (" + percentage + ")"
+}
+
+func formatPnLParts(currentValue, invested float64, formatAmount func(float64) string) (string, string) {
 	diff := currentValue - invested
 	pct := 0.0
 	if invested > 0 {
@@ -122,7 +135,7 @@ func formatPnL(currentValue, invested float64, formatAmount func(float64) string
 	if diff >= 0 {
 		sign = "+"
 	}
-	return sign + formatAmount(diff) + " (" + sign + strconv.FormatFloat(pct, 'f', 2, 64) + "%)"
+	return sign + formatAmount(diff), sign + strconv.FormatFloat(pct, 'f', 2, 64) + "%"
 }
 
 func absInt64(n int64) int64 {
diff --git a/internal/modules/stock/format_test.go b/internal/modules/stock/format_test.go
index dd5de51..55561bb 100644
--- a/internal/modules/stock/format_test.go
+++ b/internal/modules/stock/format_test.go
@@ -77,9 +77,29 @@ func TestFormatCompactVND(t *testing.T) {
 	}
 }
 
-func TestFormatPortfolioPositionPnLUsesCompactAmountAndFullPercentage(t *testing.T) {
-	if got, want := formatPortfolioPositionPnL(1_250_000, 1_000_000), "+250k (+25.00%)"; got != want {
-		t.Fatalf("formatPortfolioPositionPnL: got %q, want %q", got, want)
+func TestFormatThousandVND(t *testing.T) {
+	cases := []struct {
+		in   float64
+		want string
+	}{
+		{0, "0"},
+		{999, "0,999"},
+		{1_000, "1"},
+		{25_350, "25,35"},
+		{1_250_000, "1250"},
+		{-25_350, "-25,35"},
+	}
+	for _, c := range cases {
+		if got := formatThousandVND(c.in); got != c.want {
+			t.Errorf("formatThousandVND(%v): got %q, want %q", c.in, got, c.want)
+		}
+	}
+}
+
+func TestFormatPortfolioPositionPnLSplitsAmountAndPercentage(t *testing.T) {
+	amount, percentage := formatPortfolioPositionPnL(1_250_000, 1_000_000)
+	if amount != "+250k" || percentage != "+25.00%" {
+		t.Fatalf("formatPortfolioPositionPnL: got (%q, %q)", amount, percentage)
 	}
 }
 
diff --git a/internal/modules/stock/handlers.go b/internal/modules/stock/handlers.go
index 4da6c82..7a7a154 100644
--- a/internal/modules/stock/handlers.go
+++ b/internal/modules/stock/handlers.go
@@ -510,18 +510,19 @@ func (s *state) handleStats(ctx context.Context, b *bot.Bot, update *models.Upda
 			average := basis / float64(h.qty)
 			if !isPositiveFiniteCost(price) {
 				missingPrice = true
-				positions = append(positions, []string{h.symbol, FormatStock(float64(h.qty)), formatCompactVND(average), "N/A", "N/A", "N/A"})
+				positions = append(positions, []string{h.symbol, FormatStock(float64(h.qty)), formatThousandVND(average), "N/A", "N/A", "N/A", "N/A"})
 				continue
 			}
 			val := float64(h.qty) * price
 			if !isPositiveFiniteCost(val) || !isPositiveFiniteCost(average) {
 				missingPrice = true
-				positions = append(positions, []string{h.symbol, FormatStock(float64(h.qty)), "N/A", "N/A", "N/A", "N/A"})
+				positions = append(positions, []string{h.symbol, FormatStock(float64(h.qty)), "N/A", "N/A", "N/A", "N/A", "N/A"})
 				continue
 			}
 			totalValue += val
 			totalBasis += basis
-			positions = append(positions, []string{h.symbol, FormatStock(float64(h.qty)), formatCompactVND(average), formatCompactVND(price), formatCompactVND(val), formatPortfolioPositionPnL(val, basis)})
+			pnlAmount, pnlPercentage := formatPortfolioPositionPnL(val, basis)
+			positions = append(positions, []string{h.symbol, FormatStock(float64(h.qty)), formatThousandVND(average), formatThousandVND(price), formatCompactVND(val), pnlAmount, pnlPercentage})
 		}
 	}
 	var summary [][]string
@@ -542,7 +543,7 @@ func (s *state) handleStats(ctx context.Context, b *bot.Bot, update *models.Upda
 			{"Account P&L", formatPortfolioPnL(totalValue, p.Meta.Invested)},
 		}
 	}
-	if err := chathelper.ReplyHTML(ctx, b, update.Message, portfolioTableReply("Stock Portfolio (VND)", positions, summary)); err != nil {
+	if err := chathelper.ReplyHTML(ctx, b, update.Message, portfolioTableReply("Stock Portfolio", positions, summary)); err != nil {
 		return err
 	}
 	return s.notifyDividendEvents(ctx, b, update.Message, userID, p, checkedThrough)
@@ -558,7 +559,7 @@ func portfolioTableReply(title string, positions, summary [][]string) string {
 			rows = append(rows, []string{"… " + strconv.Itoa(omitted) + " omitted"})
 		}
 		reply := "" + title + "\n" +
-			chathelper.MonospaceTable([]string{"Ticker", "Qty", "Avg", "Now", "Value", "Unrealized P&L"}, rows) + "\n" +
+			chathelper.MonospaceTable([]string{"Sym", "Qty", "Avg", "Now", "Val", "P&L", "%"}, rows) + "\n" +
 			chathelper.MonospaceTable([]string{"Metric", "Value"}, summary)
 		if len(reply) <= portfolioReplyLimit || len(positions) == 0 {
 			return reply
diff --git a/internal/modules/stock/stats_test.go b/internal/modules/stock/stats_test.go
index a5ebc28..091a585 100644
--- a/internal/modules/stock/stats_test.go
+++ b/internal/modules/stock/stats_test.go
@@ -67,18 +67,19 @@ func TestHandleStats_UsesSSIBatchPrices(t *testing.T) {
 
 	text := rb.LastSent().Text()
 	for _, want := range []string{
-		"Stock Portfolio (VND)",
+		"Stock Portfolio",
 		"
",
-		"Ticker",
+		"Sym",
 		"MWG",
-		"60k",
-		"70k",
+		"60",
+		"70",
 		"126M",
-		"+18M (+16.67%)",
+		"+18M",
+		"+16.67%",
 		"Cash",
 		"Total value",
 		"530.335.000",
-		"Unrealized P&L",
+		"P&L",
 		"+85.000.000 (+19.19%)",
 		"Account P&L",
 		"-469.665.000 (-46.97%)",
@@ -90,8 +91,8 @@ func TestHandleStats_UsesSSIBatchPrices(t *testing.T) {
 	if strings.Contains(text, "N/A") {
 		t.Fatalf("stats rendered missing prices:\n%s", text)
 	}
-	if strings.Count(text, "VND") != 1 {
-		t.Fatalf("stats should declare VND only in the title:\n%s", text)
+	if strings.Contains(text, "VND") {
+		t.Fatalf("stock portfolio should use implicit VND:\n%s", text)
 	}
 }
 
@@ -126,13 +127,13 @@ func TestHandleStats_UnavailablePriceKeepsCompactAverage(t *testing.T) {
 	}
 
 	text := rb.LastSent().Text()
-	for _, want := range []string{"TCB", "25,35k", "N/A", "Priced value (partial)", "Account P&L", "Unavailable"} {
+	for _, want := range []string{"TCB", "25,35", "N/A", "Priced value (partial)", "Account P&L", "Unavailable"} {
 		if !strings.Contains(text, want) {
 			t.Fatalf("portfolio missing %q in:\n%s", want, text)
 		}
 	}
-	if got := strings.Count(text, "N/A"); got != 3 {
-		t.Fatalf("missing-price position has %d N/A cells, want 3:\n%s", got, text)
+	if got := strings.Count(text, "N/A"); got != 4 {
+		t.Fatalf("missing-price position has %d N/A cells, want 4:\n%s", got, text)
 	}
 }
 
@@ -171,8 +172,8 @@ func TestHandleStats_OverflowedValuationKeepsMonetaryCellsUnavailable(t *testing
 			t.Fatalf("overflow portfolio missing %q in:\n%s", want, text)
 		}
 	}
-	if got := strings.Count(text, "N/A"); got != 4 {
-		t.Fatalf("overflowed position has %d N/A monetary cells, want 4:\n%s", got, text)
+	if got := strings.Count(text, "N/A"); got != 5 {
+		t.Fatalf("overflowed position has %d N/A monetary cells, want 5:\n%s", got, text)
 	}
 	if strings.Contains(text, "1k") {
 		t.Fatalf("overflowed position exposed its average instead of N/A:\n%s", text)