From f597c0cbef035670e3a0b164246b43305c5716c9 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 9 Jun 2026 14:24:07 -0700 Subject: [PATCH] async load candles/dividends when loading portfolio --- .pre-commit-config.yaml | 8 ++ src/tui.zig | 285 +++++++++++++++++++++++++++++++++++++- src/tui/portfolio_tab.zig | 19 ++- src/tui/review_tab.zig | 97 ++++--------- 4 files changed, 330 insertions(+), 79 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 08e39ca..fb9f44a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,6 +23,14 @@ repos: entry: bash -c 'printf "%s\n" "$@" | zlint --deny-warnings --fix -S' -- language: system types: [zig] + # zlint 0.7.9 misparses Zig 0.16's `async`/`await` method + # names (Group.async, Future.await) as the legacy keywords, + # firing false-positive "syntax error" diagnostics. Until + # zlint catches up, exclude files that legitimately use the + # std.Io.Group async/await methods. `zig fmt` rewrites the + # `@"async"`/`@"await"` workaround back to the bare form, + # so we can't dodge it locally either. + exclude: ^src/tui\.zig$ - repo: https://github.com/batmac/pre-commit-zig rev: v0.3.0 hooks: diff --git a/src/tui.zig b/src/tui.zig index db71d99..a7fa435 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -473,16 +473,125 @@ pub const PortfolioData = struct { /// first consumption. prefetched_prices: ?std.StringHashMap(f64) = null, + /// Per-symbol cached candles. Populated synchronously in + /// `App.ensurePortfolioDataLoaded` — the candle map is a + /// byproduct of computing historical snapshots there, so we + /// transfer ownership to App rather than re-reading the cache + /// per-tab. Cleared on portfolio reload, freed once in + /// `deinit`. + candle_map: ?std.StringHashMap([]const zfin.Candle) = null, + /// Per-symbol cached dividends, populated by an async + /// background load started in `App.ensurePortfolioDataLoaded`. + /// Tabs that need this map MUST call `App.waitMapsReady()` + /// first; the load may still be in flight when a tab + /// activates. Each value slice is allocated by the cache + /// layer (`getCachedDividends` → `readSlice`) against the + /// App's allocator and is owned here — `PortfolioData.deinit` + /// frees both the slice and its inner currency strings via + /// `Dividend.freeSlice`. + dividend_map: ?std.StringHashMap([]const zfin.Dividend) = null, + + /// `std.Io.Group` holding the per-symbol async tasks that + /// populate `dividend_map`. Per Zig 0.16 release notes, + /// `cancel(io)` is the canonical drain primitive — used on + /// portfolio reload (the in-flight load is now stale; throw + /// it away) and on App teardown. `await(io)` is what tabs + /// use through `App.waitMapsReady()` when they actually need + /// the maps. + map_load_group: std.Io.Group = .init, + /// Per-symbol result slots written by background workers. + /// `App.waitMapsReady` folds these into `dividend_map` after + /// the Group's `await` succeeds. Tied to the Group's + /// lifetime: cleared after fold, freed on cancel. + map_load_slots: ?[]MapLoadSlot = null, + /// Coordination state for the in-flight map load. + /// `idle` = no load running, no maps. `loading` = workers + /// dispatched, slots populated, dividend_map not yet folded. + /// `ready` = `await` succeeded, dividend_map populated, slots + /// freed. `canceled` = an external cancel propagated through; + /// dividend_map stays null and tabs degrade gracefully. + map_load_phase: MapLoadPhase = .idle, + + /// Free `candle_map` and `dividend_map` and their value + /// slices. Sets both fields to null. Idempotent — safe to + /// call when one or both maps are already null. + /// + /// Used by both `deinit` (App teardown) and the portfolio + /// reload path (drop the stale maps before rebuilding). The + /// callers are responsible for canceling any in-flight + /// background load via `App.cancelMapLoad` BEFORE invoking + /// this — workers must not be holding references to slot + /// pointers when we free the maps. + pub fn freeMaps(self: *PortfolioData, allocator: std.mem.Allocator) void { + if (self.candle_map) |*m| { + var it = m.valueIterator(); + while (it.next()) |v| allocator.free(v.*); + m.deinit(); + self.candle_map = null; + } + if (self.dividend_map) |*m| { + // Each slice was allocated by the cache layer + // (getCachedDividends → readSlice) against this same + // allocator. We're the only owner; free them. + var it = m.valueIterator(); + while (it.next()) |v| zfin.Dividend.freeSlice(allocator, v.*); + m.deinit(); + self.dividend_map = null; + } + } + pub fn deinit(self: *PortfolioData, allocator: std.mem.Allocator) void { if (self.summary) |*s| s.deinit(allocator); if (self.account_map) |*am| am.deinit(); if (self.watchlist_prices) |*wp| wp.deinit(); if (self.prefetched_prices) |*pp| pp.deinit(); if (self.file) |*pf| pf.deinit(); + self.freeMaps(allocator); + if (self.map_load_slots) |slots| allocator.free(slots); self.* = .{}; } }; +/// Phase of the background candle/dividend map load. See +/// `PortfolioData.map_load_phase` for state-machine docs. +pub const MapLoadPhase = enum { + idle, + loading, + ready, + canceled, +}; + +/// Per-symbol result slot for the background dividend loader. +/// Workers write `dividends`; the App folds these into +/// `dividend_map` after `Group.await` succeeds. +/// +/// `symbol` borrows from `summary.allocations[i].symbol`, which +/// stays alive for the life of the `summary` (cleared on reload +/// only after the Group is canceled, so the borrow is safe). +pub const MapLoadSlot = struct { + symbol: []const u8, + dividends: ?[]const zfin.Dividend = null, +}; + +/// Background worker that populates one slot's `dividends` field +/// from the cache. Runs as a `std.Io.Group.async` task; signature +/// is `Cancelable!void` per the Group contract. +/// +/// Calls `io.checkCancel()` so a cancel-on-reload propagates +/// promptly even when the slot list is large. The cache read +/// itself is sync — fast enough for cancellation between symbols +/// to be the right granularity. +fn loadDividendsForSlot( + io: std.Io, + svc: *zfin.DataService, + slot: *MapLoadSlot, +) std.Io.Cancelable!void { + try io.checkCancel(); + if (svc.getCachedDividends(slot.symbol)) |divs| { + slot.dividends = divs; + } +} + /// Root widget for the interactive TUI. Implements the vaxis `vxfw.Widget` /// interface via `widget()`, which wires `typeErasedEventHandler` and /// `typeErasedDrawFn` as callbacks. Passed to `vxfw.App.run()` as the @@ -1422,7 +1531,7 @@ pub const App = struct { self.portfolio.latest_quote_date = latest_date; // Build portfolio summary, candle map, and historical snapshots - var pf_data = portfolio_loader.buildPortfolioData(self.allocator, pf, positions, syms, &prices, self.svc, self.today) catch |err| switch (err) { + const pf_data = portfolio_loader.buildPortfolioData(self.allocator, pf, positions, syms, &prices, self.svc, self.today) catch |err| switch (err) { error.NoAllocations => { self.setStatus("No cached prices. Run: zfin perf first"); return; @@ -1436,14 +1545,25 @@ pub const App = struct { return; }, }; - // Transfer ownership: summary stored on App, candle_map freed after snapshots extracted + // Transfer ownership: summary, candle_map, and snapshots + // all move onto App. The candle_map is a byproduct of the + // historical-snapshot computation that happened inside + // `buildPortfolioData`; reusing it across tabs avoids + // redundant SRF cache reads per tab activation. self.portfolio.summary = pf_data.summary; self.portfolio.historical_snapshots = pf_data.snapshots; - { - var it = pf_data.candle_map.valueIterator(); + // Free any prior candle_map before overwriting. + if (self.portfolio.candle_map) |*m| { + var it = m.valueIterator(); while (it.next()) |v| self.allocator.free(v.*); - pf_data.candle_map.deinit(); + m.deinit(); } + self.portfolio.candle_map = pf_data.candle_map; + + // Spawn the background dividend loader. Idempotent — if + // a prior load is still in flight, this is a no-op; the + // reload path is responsible for canceling first. + self.startBackgroundDividendLoad(); // Show warning if any securities failed to load if (fail_count > 0) { @@ -1499,6 +1619,95 @@ pub const App = struct { self.portfolio.summary = null; } + /// Spawn the background dividend loader. One async task per + /// symbol via `std.Io.Group.async`; the runtime decides + /// whether to actually run them concurrently. No-op if there + /// are no allocations or if a load is already in flight. + /// + /// Caller must have already populated `summary.allocations`. + /// Sets `map_load_phase = .loading` on success. + pub fn startBackgroundDividendLoad(self: *App) void { + if (self.portfolio.map_load_phase == .loading) return; + const summary = self.portfolio.summary orelse return; + if (summary.allocations.len == 0) return; + + const slots = self.allocator.alloc(MapLoadSlot, summary.allocations.len) catch return; + for (slots, summary.allocations) |*slot, alloc| { + slot.* = .{ .symbol = alloc.symbol }; + } + self.portfolio.map_load_slots = slots; + self.portfolio.map_load_phase = .loading; + + for (slots) |*slot| { + self.portfolio.map_load_group.async(self.io, loadDividendsForSlot, .{ self.io, self.svc, slot }); + } + } + + /// Block until the background dividend load completes, then + /// fold the slot results into `dividend_map`. Idempotent — + /// subsequent calls return immediately because + /// `map_load_phase` advances to `.ready` after the first fold. + /// + /// On `error.Canceled` from `Group.await` (which can only + /// happen if an external cancel propagated through, since we + /// never call `cancel` before `await`), the maps stay null + /// and `map_load_phase` becomes `.canceled`. Tabs see a null + /// map and skip the dividend-aware code path; not a fatal + /// degradation. + pub fn waitMapsReady(self: *App) void { + switch (self.portfolio.map_load_phase) { + .idle, .ready, .canceled => return, + .loading => {}, + } + + self.portfolio.map_load_group.await(self.io) catch |err| switch (err) { + error.Canceled => { + self.portfolio.map_load_phase = .canceled; + if (self.portfolio.map_load_slots) |slots| { + self.allocator.free(slots); + self.portfolio.map_load_slots = null; + } + return; + }, + }; + + // Fold slots into dividend_map. + const slots = self.portfolio.map_load_slots orelse { + self.portfolio.map_load_phase = .ready; + return; + }; + var dividends = std.StringHashMap([]const zfin.Dividend).init(self.allocator); + for (slots) |slot| { + if (slot.dividends) |d| dividends.put(slot.symbol, d) catch |err| { + std.log.scoped(.tui).warn("dividend_map.put({s}): {t}", .{ slot.symbol, err }); + }; + } + self.portfolio.dividend_map = dividends; + self.allocator.free(slots); + self.portfolio.map_load_slots = null; + self.portfolio.map_load_phase = .ready; + } + + /// Cancel any in-flight background dividend load and release + /// its resources. Used on portfolio reload (the in-flight + /// load is now stale) and on App teardown. After return, the + /// Group is in its post-cancel state — caller must + /// re-initialize via `self.portfolio.map_load_group = .init` + /// before starting a new load. + /// + /// `Group.cancel` returns `void` (unlike `Future.cancel`). + /// Workers see `error.Canceled` from any `io.checkCancel()` + /// call; our worker has one between the slot index and the + /// cache read so cancellation propagates promptly. + pub fn cancelMapLoad(self: *App) void { + self.portfolio.map_load_group.cancel(self.io); + if (self.portfolio.map_load_slots) |slots| { + self.allocator.free(slots); + self.portfolio.map_load_slots = null; + } + self.portfolio.map_load_phase = .idle; + } + pub fn setStatus(self: *App, msg: []const u8) void { const len = @min(msg.len, self.status_msg.len); @memcpy(self.status_msg[0..len], msg[0..len]); @@ -1581,6 +1790,13 @@ pub const App = struct { } fn deinitData(self: *App) void { + // Cancel the in-flight background dividend load before + // tearing anything down. Workers borrow `slot.symbol` + // pointers from `summary.allocations`, which gets freed + // by `self.portfolio.deinit` below — canceling first + // guarantees no worker can still be reading symbol + // strings during teardown. + self.cancelMapLoad(); self.symbol_data.deinit(self.allocator); // Comptime walk every tab in the registry. Hand-enumerated // lists drift — review_tab and performance_tab were silently @@ -2761,3 +2977,62 @@ test "renderBrailleToStyledLines: full price label renders for portfolios over $ try testing.expect(std.mem.indexOf(u8, rendered.items, ".89") != null); try testing.expect(std.mem.indexOf(u8, rendered.items, ",") != null); } + +// ── PortfolioData lifecycle tests ──────────────────────────── + +test "PortfolioData.deinit: empty struct cleans up without leaks" { + var pd: PortfolioData = .{}; + pd.deinit(std.testing.allocator); +} + +test "PortfolioData.deinit: with candle_map frees value slices" { + const Candle = zfin.Candle; + var pd: PortfolioData = .{}; + var cm = std.StringHashMap([]const Candle).init(std.testing.allocator); + const slice = try std.testing.allocator.alloc(Candle, 1); + slice[0] = .{ + .date = zfin.Date.fromYmd(2026, 1, 1), + .open = 1.0, + .high = 1.0, + .low = 1.0, + .close = 1.0, + .adj_close = 1.0, + .volume = 0, + }; + try cm.put("VTI", slice); + pd.candle_map = cm; + pd.deinit(std.testing.allocator); +} + +test "PortfolioData.deinit: with dividend_map frees value slices and inner currency strings" { + const Dividend = zfin.Dividend; + var pd: PortfolioData = .{}; + var dm = std.StringHashMap([]const Dividend).init(std.testing.allocator); + // Allocate a real slice with a Dividend that owns an inner + // string. PortfolioData.deinit must free both the slice and + // the string for testing.allocator to be satisfied. + const slice = try std.testing.allocator.alloc(Dividend, 1); + const owned_currency = try std.testing.allocator.dupe(u8, "USD"); + slice[0] = .{ + .ex_date = zfin.Date.fromYmd(2026, 1, 1), + .pay_date = zfin.Date.fromYmd(2026, 1, 15), + .amount = 0.5, + .currency = owned_currency, + }; + try dm.put("VTI", slice); + pd.dividend_map = dm; + pd.deinit(std.testing.allocator); +} + +test "PortfolioData.deinit: with map_load_slots frees the slot array" { + var pd: PortfolioData = .{}; + pd.map_load_slots = try std.testing.allocator.alloc(MapLoadSlot, 3); + for (pd.map_load_slots.?, 0..) |*slot, i| { + slot.* = .{ .symbol = switch (i) { + 0 => "A", + 1 => "B", + else => "C", + } }; + } + pd.deinit(std.testing.allocator); +} diff --git a/src/tui/portfolio_tab.zig b/src/tui/portfolio_tab.zig index e6a4b74..96d7fa6 100644 --- a/src/tui/portfolio_tab.zig +++ b/src/tui/portfolio_tab.zig @@ -1737,6 +1737,14 @@ pub fn reloadPortfolioFile(state: *State, app: *App) void { // Recompute summary using cached prices (no network) app.freePortfolioSummary(); + + // Cancel any in-flight background dividend load and free the + // shared candle/dividend maps before rebuilding. Re-init the + // Group for the next load cycle. + app.cancelMapLoad(); + app.portfolio.freeMaps(app.allocator); + app.portfolio.map_load_group = .init; + state.expanded = @splat(false); state.cash_expanded = false; state.illiquid_expanded = false; @@ -1779,7 +1787,7 @@ pub fn reloadPortfolioFile(state: *State, app: *App) void { app.portfolio.latest_quote_date = latest_date; // Build portfolio summary, candle map, and historical snapshots from cache - var pf_data = portfolio_loader.buildPortfolioData(app.allocator, pf, positions, syms, &prices, app.svc, app.today) catch |err| switch (err) { + const pf_data = portfolio_loader.buildPortfolioData(app.allocator, pf, positions, syms, &prices, app.svc, app.today) catch |err| switch (err) { error.NoAllocations => { app.setStatus("No cached prices available"); return; @@ -1795,11 +1803,10 @@ pub fn reloadPortfolioFile(state: *State, app: *App) void { }; app.portfolio.summary = pf_data.summary; app.portfolio.historical_snapshots = pf_data.snapshots; - { - var it = pf_data.candle_map.valueIterator(); - while (it.next()) |v| app.allocator.free(v.*); - pf_data.candle_map.deinit(); - } + // Transfer candle_map ownership to App; start background + // dividend load. Mirrors ensurePortfolioDataLoaded. + app.portfolio.candle_map = pf_data.candle_map; + app.startBackgroundDividendLoad(); sortPortfolioAllocations(state, app); buildAccountList(state, app); diff --git a/src/tui/review_tab.zig b/src/tui/review_tab.zig index 9ac0875..0d37d48 100644 --- a/src/tui/review_tab.zig +++ b/src/tui/review_tab.zig @@ -27,7 +27,6 @@ const observations = @import("../analytics/observations.zig"); const Journal = @import("../data/Journal.zig"); const input_buffer = @import("input_buffer.zig"); const portfolio_risk = @import("../analytics/portfolio_risk.zig"); -const service = @import("../service.zig"); const App = tui.App; const StyledLine = tui.StyledLine; @@ -88,9 +87,6 @@ pub const State = struct { /// lazily on first activation, kept across reloads (cheap and /// rarely-changing). Freed in `deinit`. classification_map: ?zfin.classification.ClassificationMap = null, - /// Cached dividends per symbol so we don't re-walk the cache on - /// every render. Owned by State; freed in `deinit` and `reload`. - dividend_map: ?std.StringHashMap([]const zfin.Dividend) = null, /// Active sort field. Default `.sector` (asc) provides the /// "grouped by sector with symbol-asc tiebreaker" entry state /// — see `views/review.sortRows` for why the sector column @@ -254,7 +250,6 @@ pub const tab = struct { state.view = null; if (state.findings_view) |*fv| fv.deinit(app.allocator); state.findings_view = null; - freeDividendMap(state, app); state.loaded = false; if (app.portfolio.account_map) |*am| am.deinit(); app.portfolio.account_map = null; @@ -700,48 +695,29 @@ fn loadData(state: *State, app: *App) void { app.ensureAccountMap(); - // Build dividend map from cache (no network — keeps activation fast - // on large portfolios). - if (state.dividend_map == null) { - var dm = std.StringHashMap([]const zfin.Dividend).init(app.allocator); - for (summary.allocations) |a| { - if (app.svc.getCachedDividends(a.symbol)) |divs| { - dm.put(a.symbol, divs) catch continue; - } - } - state.dividend_map = dm; - } + // Block until the background dividend load is done. Idempotent — + // first tab to call this pays the wait (typically zero by the + // time a user has clicked into the review tab); subsequent tabs + // and re-entries return instantly. + app.waitMapsReady(); - // Build a borrowed candle map: for each allocation, fetch cached - // candles. We hold the FetchResults until the view is built so - // their backing slices stay valid; releasing them via `defer` is - // safe because `buildReview` only reads the slices during its - // call and the view doesn't retain pointers into them. - var fetch_results: std.ArrayList(service.FetchResult(zfin.Candle)) = .empty; - defer { - for (fetch_results.items) |fr| fr.deinit(); - fetch_results.deinit(app.allocator); - } - var candle_map = std.StringHashMap([]const zfin.Candle).init(app.allocator); - defer candle_map.deinit(); - - for (summary.allocations) |a| { - if (app.svc.getCachedCandles(a.symbol)) |fr| { - fetch_results.append(app.allocator, fr) catch { - fr.deinit(); - continue; - }; - candle_map.put(a.symbol, fr.data) catch continue; - } - } + // Use the App-shared candle and dividend maps. Both were + // populated when the portfolio loaded (candle_map + // synchronously as a byproduct of buildPortfolioData; + // dividend_map asynchronously via the background loader). + // No per-tab cache walking. + const candle_map = if (app.portfolio.candle_map) |*m| m else { + app.setStatus("Portfolio data not loaded"); + return; + }; if (state.view) |*v| v.deinit(app.allocator); state.view = review_view.buildReview( app.allocator, app.io, summary, - &candle_map, - if (state.dividend_map) |*dm| dm else null, + candle_map, + if (app.portfolio.dividend_map) |*dm| dm else null, pf, state.classification_map orelse return, app.portfolio.account_map, @@ -834,10 +810,6 @@ fn rebuildFindingsView(state: *State, app: *App) void { } } -fn freeDividendMap(state: *State, app: *App) void { - freeDividendMapWithAllocator(state, app.allocator); -} - /// State teardown that doesn't require an App. Lets tests exercise /// the cleanup path directly under `testing.allocator`. pub fn deinitState(state: *State, allocator: std.mem.Allocator) void { @@ -847,22 +819,9 @@ pub fn deinitState(state: *State, allocator: std.mem.Allocator) void { if (state.classification_map) |*cm| cm.deinit(); for (state.note_fragments.items) |frag| allocator.free(frag); state.note_fragments.deinit(allocator); - freeDividendMapWithAllocator(state, allocator); state.* = .{}; } -fn freeDividendMapWithAllocator(state: *State, allocator: std.mem.Allocator) void { - const dm_opt = &state.dividend_map; - if (dm_opt.* == null) return; - var dm = dm_opt.*.?; - var it = dm.iterator(); - while (it.next()) |entry| { - zfin.Dividend.freeSlice(allocator, @constCast(entry.value_ptr.*)); - } - dm.deinit(); - dm_opt.* = null; -} - // ── Rendering ───────────────────────────────────────────────── /// Column widths in display-column order. Tax% is LAST: it's a @@ -2017,10 +1976,9 @@ test "handleAction: sort_reverse flips direction" { try testing.expectEqual(review_view.SortDirection.asc, state.sort_dir); } -test "deinitState: cleans up view + classification_map + dividend_map (leak check)" { - // Allocate a real ReviewView + populate dividend_map with allocated - // slices. deinitState must free everything for testing.allocator - // to pass. +test "deinitState: cleans up view (leak check)" { + // Allocate a real ReviewView. deinitState must free everything + // for testing.allocator to pass. const Date = zfin.Date; var allocs = [_]@import("../analytics/valuation.zig").Allocation{ @@ -2072,10 +2030,10 @@ test "deinitState: cleans up view + classification_map + dividend_map (leak chec "test.srf", ); - // Build a dividend map with one allocated entry to exercise the - // freeSlice path in deinit. The slice is a single Dividend record; - // freeSlice frees both the inner allocations (none here, just - // primitive fields) and the outer slice. + // Build a dividend map with one allocated entry — but DO NOT + // attach it to State (dividend_map lives on App now). Free it + // after deinitState so the test's testing.allocator stays + // satisfied. var dividend_map = std.StringHashMap([]const zfin.Dividend).init(testing.allocator); const divs = try testing.allocator.alloc(zfin.Dividend, 1); divs[0] = .{ .ex_date = Date.fromYmd(2024, 6, 15), .pay_date = Date.fromYmd(2024, 7, 1), .amount = 1.5 }; @@ -2085,15 +2043,18 @@ test "deinitState: cleans up view + classification_map + dividend_map (leak chec .loaded = true, .view = view, .classification_map = null, // owned by caller in this test - .dividend_map = dividend_map, .sort_field = .sector, .sort_dir = .asc, }; - // Single deinit must free everything we allocated above. + // Single deinit must free everything State owns. deinitState(&state, testing.allocator); try testing.expect(state.view == null); - try testing.expect(state.dividend_map == null); + + // Manually clean up our standalone dividend_map (not owned by + // State — App owns these in production). + testing.allocator.free(divs); + dividend_map.deinit(); } // ── Step 8a observations integration tests ─────────────────────