treat candles separately for great good

src/PortfolioData.zig

@@ -222,6 +222,20 @@ pub const WorkerDelays = struct {
 // "ready right now" (field access) vs. "may block" (method call).
 
 arena: ArenaAllocator,
+
+/// Dedicated arena for `candles_data` (the map storage, the
+/// duped symbol keys, and the candle slices themselves). Lives
+/// ACROSS reloads — only reset on `force_refresh` or `deinit`.
+/// This lets a soft reload reuse already-loaded candles instead
+/// of re-reading the cache for every held symbol (~870ms warm-
+/// cache cost dominated reload time before this).
+///
+/// Removed-from-portfolio symbols' slices stay in this arena
+/// until the next `force_refresh` or `deinit` — accepted
+/// trade-off (a few hundred KB at worst over a session) for
+/// O(1) bulk free and no per-entry tracking.
+candles_arena: ArenaAllocator,
+
 io: std.Io,
 svc: *DataService,
 
@@ -256,6 +270,11 @@ watchlist_prices: ?std.StringHashMap(f64) = null,
 // the methods.
 
 candles_future: ?std.Io.Future(void) = null,
+/// Per-symbol candle slices. Backed by `candles_arena`, so this
+/// map and its contents survive `arena.reset()` on reload — the
+/// candles worker only loads symbols that aren't already in the
+/// map. Reset wholesale on `force_refresh` (via
+/// `candles_arena.reset`) or on `deinit`.
 candles_data: ?std.StringHashMap([]const Candle) = null,
 
 snapshots_future: ?std.Io.Future(void) = null,
@@ -272,16 +291,21 @@ account_map_data: ?AccountMap = null,
 pub fn init(opts: InitOptions) PortfolioData {
     return .{
         .arena = .init(opts.gpa),
+        .candles_arena = .init(opts.gpa),
         .io = opts.io,
         .svc = opts.svc,
     };
 }
 
 /// Tear down. Cancels any in-flight async work, frees the
-/// parsed file, releases the arena's pages back to the GPA.
+/// parsed file, releases both arenas' pages back to the GPA.
 pub fn deinit(self: *PortfolioData) void {
     self.cancelLoad();
     if (self.file) |*pf| pf.deinit();
+    // candles_arena owns the candle map's storage, keys, and
+    // slice values — single bulk free is enough; no need to
+    // walk the map.
+    self.candles_arena.deinit();
     self.arena.deinit();
     self.* = undefined;
 }
@@ -401,11 +425,18 @@ pub fn load(
     self.summary = null;
     self.latest_quote_date = null;
     self.watchlist_prices = null;
-    self.candles_data = null;
     self.snapshots_data = null;
     self.dividends_data = null;
     self.account_map_data = null;
 
+    // candles_data lives in candles_arena and survives across
+    // reloads — kept entries are reused, only new symbols hit
+    // the cache. force_refresh wipes it wholesale.
+    if (opts.force_refresh) {
+        _ = self.candles_arena.reset(.retain_capacity);
+        self.candles_data = null;
+    }
+
     if (paths_in.len == 0) return error.NoPaths;
 
     // Capture paths in the arena so reload() can re-use them.
@@ -543,7 +574,29 @@ pub fn load(
     }
     const positions_arena = dupePositions(arena_alloc, positions);
 
-    self.candles_future = self.io.async(candlesWorker, .{ self, syms_arena, opts.delays.candles_ms });
+    // Build the candles to-load list: symbols not already in
+    // candles_data. On a soft reload with no symbol changes,
+    // this is empty and the worker exits ~instantly. On a fresh
+    // load (or force_refresh), this is the full symbol set.
+    // Allocated against the per-load arena — used only for the
+    // duration of the worker's run.
+    const candles_to_load = blk: {
+        if (self.candles_data) |*existing| {
+            const dst = arena_alloc.alloc([]const u8, syms_arena.len) catch
+                return error.OutOfMemory;
+            var n: usize = 0;
+            for (syms_arena) |s| {
+                if (!existing.contains(s)) {
+                    dst[n] = s;
+                    n += 1;
+                }
+            }
+            break :blk dst[0..n];
+        }
+        break :blk syms_arena;
+    };
+
+    self.candles_future = self.io.async(candlesWorker, .{ self, candles_to_load, opts.delays.candles_ms });
     self.snapshots_future = self.io.async(snapshotsWorker, .{ self, today, positions_arena, opts.delays.snapshots_ms });
     self.dividends_future = self.io.async(dividendsWorker, .{ self, opts.delays.dividends_ms });
     self.account_map_future = self.io.async(accountMapWorker, .{ self, opts.delays.account_map_ms });
@@ -582,16 +635,26 @@ pub fn reload(self: *PortfolioData, today: Date, opts: LoadOptions) LoadError!Lo
 
 /// Cancel any in-flight load and pending background workers.
 /// Safe to call at any time including when nothing is in-flight.
-/// After cancel, every async data field is null, so subsequent
+/// After cancel, snapshots / dividends / account_map data is
-/// method calls (`pd.candles()` etc.) return null without
+/// nulled — `pd.snapshots()` etc. return null without blocking.
-/// blocking.
+///
+/// `candles_data` is intentionally NOT nulled: any partial
+/// entries the candles worker managed to populate before
+/// cancel are valid (they came from successful cache reads),
+/// and they'll be reused on the next load. The candles_arena
+/// retains them; the next force_refresh or deinit reaps.
+///
+/// Cancellation order matters: snapshots depends on candles
+/// (snapshotsWorker internally awaits the candles future).
+/// Cancel snapshots FIRST so its worker exits before we
+/// cancel the candles future — otherwise we'd race `cancel`
+/// against `await` on the same future.
 pub fn cancelLoad(self: *PortfolioData) void {
-    if (self.candles_future) |*f| _ = f.cancel(self.io);
-    self.candles_future = null;
-    self.candles_data = null;
     if (self.snapshots_future) |*f| _ = f.cancel(self.io);
     self.snapshots_future = null;
     self.snapshots_data = null;
+    if (self.candles_future) |*f| _ = f.cancel(self.io);
+    self.candles_future = null;
     if (self.dividends_future) |*f| _ = f.cancel(self.io);
     self.dividends_future = null;
     self.dividends_data = null;
@@ -613,17 +676,26 @@ pub fn cancelLoad(self: *PortfolioData) void {
 // Callers are expected to handle null gracefully (degrade UX,
 // not crash).
 
-fn candlesWorker(self: *PortfolioData, syms: []const []const u8, delay_ms: usize) void {
+fn candlesWorker(self: *PortfolioData, to_load: []const []const u8, delay_ms: usize) void {
     self.io.sleep(.fromMilliseconds(@intCast(delay_ms)), .real) catch return;
-    const arena_alloc = self.allocator();
+
-    var map = std.StringHashMap([]const Candle).init(arena_alloc);
+    // Lazy-init the persistent candle map on first use.
-    for (syms) |sym| {
+    // Subsequent loads find it non-null and append.
+    const candles_alloc = self.candles_arena.allocator();
+    if (self.candles_data == null) {
+        self.candles_data = std.StringHashMap([]const Candle).init(candles_alloc);
+    }
+    var map = &self.candles_data.?;
+
+    for (to_load) |sym| {
         self.io.checkCancel() catch return;
-        if (self.svc.getCachedCandles(arena_alloc, sym)) |cs| {
+        if (self.svc.getCachedCandles(candles_alloc, sym)) |cs| {
-            map.put(sym, cs.data) catch continue;
+            // Dupe the symbol key into candles_arena so it
+            // outlives the per-load arena that owns `to_load`.
+            const key = candles_alloc.dupe(u8, sym) catch continue;
+            map.put(key, cs.data) catch continue;
         }
     }
-    self.candles_data = map;
 }
 
 fn snapshotsWorker(self: *PortfolioData, as_of: Date, positions: []const zfin.Position, delay_ms: usize) void {
@@ -745,7 +817,11 @@ test "PortfolioData.cancelLoad: idempotent on idle state" {
 
     pd.cancelLoad();
     pd.cancelLoad(); // second time is a no-op
-    // After cancel, every worker future and async data field is nulled.
+    // After cancel: futures cleared; per-load data fields
+    // (snapshots, dividends, account_map) nulled. candles_data
+    // is intentionally NOT cleared by cancel — it persists
+    // across reloads. Idle pd has it null because nothing has
+    // ever populated it.
     try testing.expect(pd.candles_future == null);
     try testing.expect(pd.candles_data == null);
     try testing.expect(pd.dividends_future == null);
@@ -904,3 +980,103 @@ test "PortfolioData.WorkerDelays: defaults are all zero" {
     try testing.expectEqual(@as(usize, 0), d.dividends_ms);
     try testing.expectEqual(@as(usize, 0), d.account_map_ms);
 }
+
+// ── Persistent candle cache tests ────────────────────────────
+//
+// candles_data lives in candles_arena and survives reloads.
+// These tests white-box that property without spinning up a
+// real DataService — pre-populating the map directly and
+// inspecting state.
+
+test "PortfolioData.candles_data: deinit frees candles_arena cleanly" {
+    var svc: DataService = .{
+        .allocator = testing.allocator,
+        .io = testing.io,
+        .config = .{ .cache_dir = "./.tmp/zfin-pd-test-cache" },
+    };
+    var pd = PortfolioData.init(.{
+        .gpa = testing.allocator,
+        .io = testing.io,
+        .svc = &svc,
+    });
+
+    // Populate candles_data manually using candles_arena. With
+    // testing.allocator's leak detection, deinit must reclaim
+    // every byte.
+    const ca = pd.candles_arena.allocator();
+    pd.candles_data = std.StringHashMap([]const Candle).init(ca);
+    const key1 = try ca.dupe(u8, "VTI");
+    const key2 = try ca.dupe(u8, "BND");
+    const slice1 = try ca.alloc(Candle, 2);
+    slice1[0] = .{
+        .date = Date.fromYmd(2026, 1, 1),
+        .open = 1.0,
+        .high = 1.0,
+        .low = 1.0,
+        .close = 1.0,
+        .adj_close = 1.0,
+        .volume = 0,
+    };
+    slice1[1] = slice1[0];
+    const slice2 = try ca.alloc(Candle, 1);
+    slice2[0] = slice1[0];
+    try pd.candles_data.?.put(key1, slice1);
+    try pd.candles_data.?.put(key2, slice2);
+
+    pd.deinit(); // must not leak
+}
+
+test "PortfolioData.candles_data: force_refresh resets the candles arena" {
+    var svc: DataService = .{
+        .allocator = testing.allocator,
+        .io = testing.io,
+        .config = .{ .cache_dir = "./.tmp/zfin-pd-test-cache" },
+    };
+    var pd = PortfolioData.init(.{
+        .gpa = testing.allocator,
+        .io = testing.io,
+        .svc = &svc,
+    });
+    defer pd.deinit();
+
+    // Pre-populate.
+    const ca = pd.candles_arena.allocator();
+    pd.candles_data = std.StringHashMap([]const Candle).init(ca);
+    const key = try ca.dupe(u8, "VTI");
+    const slice = try ca.alloc(Candle, 0);
+    try pd.candles_data.?.put(key, slice);
+    try testing.expect(pd.candles_data != null);
+
+    // load() with force_refresh=true and empty paths should
+    // wipe candles_data before failing on NoPaths. The
+    // candles_arena is reset, candles_data is nulled.
+    try testing.expectError(error.NoPaths, pd.load(&.{}, Date.fromYmd(2026, 1, 1), .{ .force_refresh = true }));
+    try testing.expect(pd.candles_data == null);
+}
+
+test "PortfolioData.candles_data: soft load preserves the candle map" {
+    var svc: DataService = .{
+        .allocator = testing.allocator,
+        .io = testing.io,
+        .config = .{ .cache_dir = "./.tmp/zfin-pd-test-cache" },
+    };
+    var pd = PortfolioData.init(.{
+        .gpa = testing.allocator,
+        .io = testing.io,
+        .svc = &svc,
+    });
+    defer pd.deinit();
+
+    // Pre-populate.
+    const ca = pd.candles_arena.allocator();
+    pd.candles_data = std.StringHashMap([]const Candle).init(ca);
+    const key = try ca.dupe(u8, "VTI");
+    const slice = try ca.alloc(Candle, 0);
+    try pd.candles_data.?.put(key, slice);
+
+    // Soft load with empty paths fails on NoPaths but does NOT
+    // touch candles_data (force_refresh = false).
+    try testing.expectError(error.NoPaths, pd.load(&.{}, Date.fromYmd(2026, 1, 1), .{}));
+    try testing.expect(pd.candles_data != null);
+    try testing.expect(pd.candles_data.?.contains("VTI"));
+}