consolidate shared portfolio summary calculations

src/commands/common.zig

@@ -123,6 +123,77 @@ pub const LoadProgress = struct {
     }
 };
 
+// ── Portfolio data pipeline ──────────────────────────────────
+
+/// Result of the shared portfolio data pipeline. Caller must call deinit().
+pub const PortfolioData = struct {
+    summary: zfin.valuation.PortfolioSummary,
+    candle_map: std.StringHashMap([]const zfin.Candle),
+    snapshots: ?[6]zfin.valuation.HistoricalSnapshot,
+
+    pub fn deinit(self: *PortfolioData, allocator: std.mem.Allocator) void {
+        self.summary.deinit(allocator);
+        var it = self.candle_map.valueIterator();
+        while (it.next()) |v| allocator.free(v.*);
+        self.candle_map.deinit();
+    }
+};
+
+/// Build portfolio summary, candle map, and historical snapshots from
+/// pre-populated prices. Shared between CLI `portfolio` command, TUI
+/// `loadPortfolioData`, and TUI `reloadPortfolioFile`.
+///
+/// Callers are responsible for populating `prices` (via network fetch,
+/// cache read, or pre-fetched map) before calling this.
+///
+/// Returns error.NoAllocations if the summary produces no positions
+/// (e.g. no cached prices available).
+pub fn buildPortfolioData(
+    allocator: std.mem.Allocator,
+    portfolio: zfin.Portfolio,
+    positions: []const zfin.Position,
+    syms: []const []const u8,
+    prices: *std.StringHashMap(f64),
+    svc: *zfin.DataService,
+) !PortfolioData {
+    var manual_price_set = try zfin.valuation.buildFallbackPrices(allocator, portfolio.lots, positions, prices);
+    defer manual_price_set.deinit();
+
+    var summary = zfin.valuation.portfolioSummary(allocator, portfolio, positions, prices.*, manual_price_set) catch
+        return error.SummaryFailed;
+    errdefer summary.deinit(allocator);
+
+    if (summary.allocations.len == 0) {
+        summary.deinit(allocator);
+        return error.NoAllocations;
+    }
+
+    var candle_map = std.StringHashMap([]const zfin.Candle).init(allocator);
+    errdefer {
+        var it = candle_map.valueIterator();
+        while (it.next()) |v| allocator.free(v.*);
+        candle_map.deinit();
+    }
+    for (syms) |sym| {
+        if (svc.getCachedCandles(sym)) |cs| {
+            candle_map.put(sym, cs) catch {};
+        }
+    }
+
+    const snapshots = zfin.valuation.computeHistoricalSnapshots(
+        fmt.todayDate(),
+        positions,
+        prices.*,
+        candle_map,
+    );
+
+    return .{
+        .summary = summary,
+        .candle_map = candle_map,
+        .snapshots = snapshots,
+    };
+}
+
 // ── Watchlist loading ────────────────────────────────────────
 
 /// Load a watchlist file using the library's SRF deserializer.

src/commands/portfolio.zig

@@ -100,38 +100,23 @@ pub fn run(allocator: std.mem.Allocator, config: zfin.Config, svc: *zfin.DataSer
         }
     }
 
-    // Compute summary
+    // Build portfolio summary, candle map, and historical snapshots
-    // Build fallback prices for symbols that failed API fetch
+    var pf_data = cli.buildPortfolioData(allocator, portfolio, positions, syms, &prices, svc) catch |err| switch (err) {
-    var manual_price_set = try zfin.valuation.buildFallbackPrices(allocator, portfolio.lots, positions, &prices);
+        error.NoAllocations, error.SummaryFailed => {
-    defer manual_price_set.deinit();
-
-    var summary = zfin.valuation.portfolioSummary(allocator, portfolio, positions, prices, manual_price_set) catch {
             try cli.stderrPrint("Error computing portfolio summary.\n");
             return;
+        },
+        else => return err,
     };
-    defer summary.deinit(allocator);
+    defer pf_data.deinit(allocator);
 
     // Sort allocations alphabetically by symbol
-    std.mem.sort(zfin.valuation.Allocation, summary.allocations, {}, struct {
+    std.mem.sort(zfin.valuation.Allocation, pf_data.summary.allocations, {}, struct {
         fn f(_: void, a: zfin.valuation.Allocation, b: zfin.valuation.Allocation) bool {
             return std.mem.lessThan(u8, a.display_symbol, b.display_symbol);
         }
     }.f);
 
-    // Build candle map once for historical snapshots and risk metrics.
-    // This avoids parsing the full candle history multiple times.
-    var candle_map = std.StringHashMap([]const zfin.Candle).init(allocator);
-    defer {
-        var it = candle_map.valueIterator();
-        while (it.next()) |v| allocator.free(v.*);
-        candle_map.deinit();
-    }
-    for (syms) |sym| {
-        if (svc.getCachedCandles(sym)) |cs| {
-            try candle_map.put(sym, cs);
-        }
-    }
-
     // Collect watch symbols and their prices for display.
     // Includes watch lots from portfolio + symbols from separate watchlist file.
     var watch_list: std.ArrayList([]const u8) = .empty;
@@ -142,7 +127,7 @@ pub fn run(allocator: std.mem.Allocator, config: zfin.Config, svc: *zfin.DataSer
         var watch_seen = std.StringHashMap(void).init(allocator);
         defer watch_seen.deinit();
         // Exclude portfolio position symbols from watchlist
-        for (summary.allocations) |a| {
+        for (pf_data.summary.allocations) |a| {
             try watch_seen.put(a.symbol, {});
         }
 
@@ -183,9 +168,7 @@ pub fn run(allocator: std.mem.Allocator, config: zfin.Config, svc: *zfin.DataSer
         file_path,
         &portfolio,
         positions,
-        &summary,
+        &pf_data,
-        prices,
-        candle_map,
         watch_list.items,
         watch_prices,
     );
@@ -199,12 +182,11 @@ pub fn display(
     file_path: []const u8,
     portfolio: *const zfin.Portfolio,
     positions: []const zfin.Position,
-    summary: *const zfin.valuation.PortfolioSummary,
+    pf_data: *const cli.PortfolioData,
-    prices: std.StringHashMap(f64),
-    candle_map: std.StringHashMap([]const zfin.Candle),
     watch_symbols: []const []const u8,
     watch_prices: std.StringHashMap(f64),
 ) !void {
+    const summary = &pf_data.summary;
     // Header with summary
     try cli.setBold(out, color);
     try out.print("\nPortfolio Summary ({s})\n", .{file_path});
@@ -241,13 +223,7 @@ pub fn display(
 
     // Historical portfolio value snapshots
     {
-        if (candle_map.count() > 0) {
+        if (pf_data.snapshots) |snapshots| {
-            const snapshots = zfin.valuation.computeHistoricalSnapshots(
-                fmt.todayDate(),
-                positions,
-                prices,
-                candle_map,
-            );
             try out.print("  Historical: ", .{});
             try cli.setFg(out, color, cli.CLR_MUTED);
             for (zfin.valuation.HistoricalPeriod.all, 0..) |period, pi| {
@@ -610,7 +586,7 @@ pub fn display(
         var any_risk = false;
 
         for (summary.allocations) |a| {
-            if (candle_map.get(a.symbol)) |candles| {
+            if (pf_data.candle_map.get(a.symbol)) |candles| {
                 const tr = zfin.risk.trailingRisk(candles);
                 if (tr.three_year) |metrics| {
                     if (!any_risk) {
@@ -707,6 +683,14 @@ fn testSummary(allocations: []zfin.valuation.Allocation) zfin.valuation.Portfoli
     };
 }
 
+fn testPortfolioData(summary: zfin.valuation.PortfolioSummary, candle_map: std.StringHashMap([]const zfin.Candle)) cli.PortfolioData {
+    return .{
+        .summary = summary,
+        .candle_map = candle_map,
+        .snapshots = null,
+    };
+}
+
 test "display shows header and summary" {
     var buf: [8192]u8 = undefined;
     var w: std.Io.Writer = .fixed(&buf);
@@ -726,7 +710,7 @@ test "display shows header and summary" {
         .{ .symbol = "AAPL", .display_symbol = "AAPL", .shares = 10, .avg_cost = 150.0, .current_price = 175.0, .market_value = 1750.0, .cost_basis = 1500.0, .weight = 0.745, .unrealized_gain_loss = 250.0, .unrealized_return = 0.167 },
         .{ .symbol = "GOOG", .display_symbol = "GOOG", .shares = 5, .avg_cost = 120.0, .current_price = 140.0, .market_value = 700.0, .cost_basis = 600.0, .weight = 0.255, .unrealized_gain_loss = 100.0, .unrealized_return = 0.167 },
     };
-    var summary = testSummary(&allocs);
+    const summary = testSummary(&allocs);
 
     var prices = std.StringHashMap(f64).init(testing.allocator);
     defer prices.deinit();
@@ -735,13 +719,14 @@ test "display shows header and summary" {
 
     var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
     defer candle_map.deinit();
+    const pf_data = testPortfolioData(summary, candle_map);
 
     var watch_prices = std.StringHashMap(f64).init(testing.allocator);
     defer watch_prices.deinit();
 
     const watch_syms: []const []const u8 = &.{};
 
-    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &summary, prices, candle_map, watch_syms, watch_prices);
+    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &pf_data, watch_syms, watch_prices);
     const out = w.buffered();
 
     // Header present
@@ -776,7 +761,7 @@ test "display with watchlist" {
     var allocs = [_]zfin.valuation.Allocation{
         .{ .symbol = "VTI", .display_symbol = "VTI", .shares = 20, .avg_cost = 200.0, .current_price = 220.0, .market_value = 4400.0, .cost_basis = 4000.0, .weight = 1.0, .unrealized_gain_loss = 400.0, .unrealized_return = 0.1 },
     };
-    var summary = testSummary(&allocs);
+    const summary = testSummary(&allocs);
 
     var prices = std.StringHashMap(f64).init(testing.allocator);
     defer prices.deinit();
@@ -784,6 +769,7 @@ test "display with watchlist" {
 
     var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
     defer candle_map.deinit();
+    const pf_data = testPortfolioData(summary, candle_map);
 
     // Watchlist with prices
     const watch_syms: []const []const u8 = &.{ "TSLA", "NVDA" };
@@ -792,7 +778,7 @@ test "display with watchlist" {
     try watch_prices.put("TSLA", 250.50);
     try watch_prices.put("NVDA", 800.25);
 
-    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &summary, prices, candle_map, watch_syms, watch_prices);
+    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &pf_data, watch_syms, watch_prices);
     const out = w.buffered();
 
     // Watchlist header and symbols
@@ -829,11 +815,12 @@ test "display with options section" {
 
     var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
     defer candle_map.deinit();
+    const pf_data = testPortfolioData(summary, candle_map);
     var watch_prices = std.StringHashMap(f64).init(testing.allocator);
     defer watch_prices.deinit();
     const watch_syms: []const []const u8 = &.{};
 
-    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &summary, prices, candle_map, watch_syms, watch_prices);
+    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &pf_data, watch_syms, watch_prices);
     const out = w.buffered();
 
     // Options section present
@@ -870,11 +857,12 @@ test "display with CDs and cash" {
 
     var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
     defer candle_map.deinit();
+    const pf_data = testPortfolioData(summary, candle_map);
     var watch_prices = std.StringHashMap(f64).init(testing.allocator);
     defer watch_prices.deinit();
     const watch_syms: []const []const u8 = &.{};
 
-    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &summary, prices, candle_map, watch_syms, watch_prices);
+    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &pf_data, watch_syms, watch_prices);
     const out = w.buffered();
 
     // CDs section present
@@ -913,11 +901,12 @@ test "display realized PnL shown when nonzero" {
 
     var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
     defer candle_map.deinit();
+    const pf_data = testPortfolioData(summary, candle_map);
     var watch_prices = std.StringHashMap(f64).init(testing.allocator);
     defer watch_prices.deinit();
     const watch_syms: []const []const u8 = &.{};
 
-    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &summary, prices, candle_map, watch_syms, watch_prices);
+    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &pf_data, watch_syms, watch_prices);
     const out = w.buffered();
 
     try testing.expect(std.mem.indexOf(u8, out, "Realized P&L") != null);
@@ -939,7 +928,7 @@ test "display empty watchlist not shown" {
     var allocs = [_]zfin.valuation.Allocation{
         .{ .symbol = "VTI", .display_symbol = "VTI", .shares = 10, .avg_cost = 200.0, .current_price = 220.0, .market_value = 2200.0, .cost_basis = 2000.0, .weight = 1.0, .unrealized_gain_loss = 200.0, .unrealized_return = 0.1 },
     };
-    var summary = testSummary(&allocs);
+    const summary = testSummary(&allocs);
 
     var prices = std.StringHashMap(f64).init(testing.allocator);
     defer prices.deinit();
@@ -947,11 +936,12 @@ test "display empty watchlist not shown" {
 
     var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
     defer candle_map.deinit();
+    const pf_data = testPortfolioData(summary, candle_map);
     var watch_prices = std.StringHashMap(f64).init(testing.allocator);
     defer watch_prices.deinit();
     const watch_syms: []const []const u8 = &.{};
 
-    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &summary, prices, candle_map, watch_syms, watch_prices);
+    try display(testing.allocator, &w, false, "test.srf", &portfolio, &positions, &pf_data, watch_syms, watch_prices);
     const out = w.buffered();
 
     // Watchlist header should NOT appear when there are no watch symbols

src/tui/portfolio_tab.zig

@@ -151,50 +151,35 @@ pub fn loadPortfolioData(self: *App) void {
     }
     self.candle_last_date = latest_date;
 
-    // Build fallback prices for symbols that failed API fetch
+    // Build portfolio summary, candle map, and historical snapshots
-    var manual_price_set = zfin.valuation.buildFallbackPrices(self.allocator, pf.lots, positions, &prices) catch {
+    var pf_data = cli.buildPortfolioData(self.allocator, pf, positions, syms, &prices, self.svc) catch |err| switch (err) {
-        self.setStatus("Error building fallback prices");
+        error.NoAllocations => {
-        return;
-    };
-    defer manual_price_set.deinit();
-
-    var summary = zfin.valuation.portfolioSummary(self.allocator, pf, positions, prices, manual_price_set) catch {
-        self.setStatus("Error computing portfolio summary");
-        return;
-    };
-
-    if (summary.allocations.len == 0) {
-        summary.deinit(self.allocator);
             self.setStatus("No cached prices. Run: zfin perf <SYMBOL> first");
             return;
-    }
+        },
-
+        error.SummaryFailed => {
-    self.portfolio_summary = summary;
+            self.setStatus("Error computing portfolio summary");
-
+            return;
-    // Compute historical portfolio snapshots from cached candle data
+        },
+        else => {
+            self.setStatus("Error building portfolio data");
+            return;
+        },
+    };
+    // Transfer ownership: summary stored on App, candle_map freed after snapshots extracted
+    self.portfolio_summary = pf_data.summary;
+    self.historical_snapshots = pf_data.snapshots;
     {
-        var candle_map = std.StringHashMap([]const zfin.Candle).init(self.allocator);
+        // Free candle_map values and map (snapshots are value types, already copied)
-        defer {
+        var it = pf_data.candle_map.valueIterator();
-            var it = candle_map.valueIterator();
         while (it.next()) |v| self.allocator.free(v.*);
-            candle_map.deinit();
+        pf_data.candle_map.deinit();
-        }
-        for (syms) |sym| {
-            if (self.svc.getCachedCandles(sym)) |cs| {
-                candle_map.put(sym, cs) catch {};
-            }
-        }
-        self.historical_snapshots = zfin.valuation.computeHistoricalSnapshots(
-            fmt.todayDate(),
-            positions,
-            prices,
-            candle_map,
-        );
     }
 
     sortPortfolioAllocations(self);
     rebuildPortfolioRows(self);
 
+    const summary = pf_data.summary;
     if (self.symbol.len == 0 and summary.allocations.len > 0) {
         self.setActiveSymbol(summary.allocations[0].symbol);
     }
@@ -1000,45 +985,27 @@ pub fn reloadPortfolioFile(self: *App) void {
     }
     self.candle_last_date = latest_date;
 
-    // Build fallback prices for reload path
+    // Build portfolio summary, candle map, and historical snapshots from cache
-    var manual_price_set = zfin.valuation.buildFallbackPrices(self.allocator, pf.lots, positions, &prices) catch {
+    var pf_data = cli.buildPortfolioData(self.allocator, pf, positions, syms, &prices, self.svc) catch |err| switch (err) {
-        self.setStatus("Error building fallback prices");
+        error.NoAllocations => {
-        return;
-    };
-    defer manual_price_set.deinit();
-
-    var summary = zfin.valuation.portfolioSummary(self.allocator, pf, positions, prices, manual_price_set) catch {
-        self.setStatus("Error computing portfolio summary");
-        return;
-    };
-
-    if (summary.allocations.len == 0) {
-        summary.deinit(self.allocator);
             self.setStatus("No cached prices available");
             return;
-    }
+        },
-
+        error.SummaryFailed => {
-    self.portfolio_summary = summary;
+            self.setStatus("Error computing portfolio summary");
-
+            return;
-    // Compute historical snapshots from cache (reload path)
+        },
+        else => {
+            self.setStatus("Error building portfolio data");
+            return;
+        },
+    };
+    self.portfolio_summary = pf_data.summary;
+    self.historical_snapshots = pf_data.snapshots;
     {
-        var candle_map = std.StringHashMap([]const zfin.Candle).init(self.allocator);
+        var it = pf_data.candle_map.valueIterator();
-        defer {
-            var it = candle_map.valueIterator();
         while (it.next()) |v| self.allocator.free(v.*);
-            candle_map.deinit();
+        pf_data.candle_map.deinit();
-        }
-        for (syms) |sym| {
-            if (self.svc.getCachedCandles(sym)) |cs| {
-                candle_map.put(sym, cs) catch {};
-            }
-        }
-        self.historical_snapshots = zfin.valuation.computeHistoricalSnapshots(
-            fmt.todayDate(),
-            positions,
-            prices,
-            candle_map,
-        );
     }
 
     sortPortfolioAllocations(self);