From b3ebc3f98632487bed541dc34e086da001619cce Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Thu, 23 Apr 2026 01:25:00 -0700 Subject: [PATCH] dedup the snapshot loading function --- src/commands/history.zig | 24 ++++++-------- src/history.zig | 68 ++++++++++++++++++++++++++++++++++++++++ src/tui.zig | 8 +++-- src/tui/history_tab.zig | 61 +++++++++++++---------------------- 4 files changed, 105 insertions(+), 56 deletions(-) diff --git a/src/commands/history.zig b/src/commands/history.zig index 7603f57..6ee64fa 100644 --- a/src/commands/history.zig +++ b/src/commands/history.zig @@ -180,29 +180,25 @@ fn runPortfolio( color: bool, out: *std.Io.Writer, ) !void { - // Derive history/ from the portfolio path's directory. - const portfolio_dir = std.fs.path.dirname(portfolio_path) orelse "."; - const history_dir = try std.fs.path.join(allocator, &.{ portfolio_dir, "history" }); - defer allocator.free(history_dir); - - var loaded = try history_io.loadHistoryDir(allocator, history_dir); - defer loaded.deinit(); + // Derive history/ + load snapshots + build series in one shot. + // Both --rebuild-rollup (wants raw snapshots) and the main render + // path (wants the series) share this, so the combined loader gives + // us both without re-parsing. + var tl = try history_io.loadTimeline(allocator, portfolio_path); + defer tl.deinit(); if (opts.rebuild_rollup) { - try rebuildRollup(allocator, history_dir, loaded.snapshots, out); + try rebuildRollup(allocator, tl.history_dir, tl.loaded.snapshots, out); return; } - if (loaded.snapshots.len == 0) { - try out.print("No portfolio snapshots found in {s}.\n", .{history_dir}); + if (tl.loaded.snapshots.len == 0) { + try out.print("No portfolio snapshots found in {s}.\n", .{tl.history_dir}); try out.print("Run `zfin snapshot` to capture the first one.\n", .{}); return; } - const series = try timeline.buildSeries(allocator, loaded.snapshots); - defer series.deinit(); - - const filtered = try timeline.filterByDate(allocator, series.points, opts.since, opts.until); + const filtered = try timeline.filterByDate(allocator, tl.series.points, opts.since, opts.until); defer allocator.free(filtered); if (filtered.len == 0) { diff --git a/src/history.zig b/src/history.zig index 074da6e..6cfa6d0 100644 --- a/src/history.zig +++ b/src/history.zig @@ -26,6 +26,7 @@ const std = @import("std"); const srf = @import("srf"); const snapshot_mod = @import("models/snapshot.zig"); const Date = @import("models/date.zig").Date; +const timeline = @import("analytics/timeline.zig"); pub const Error = error{ /// The file didn't open a `#!srfv1` directive or couldn't be @@ -207,6 +208,73 @@ pub fn loadHistoryDir( }; } +/// Derive `/history` and return the joined +/// path (caller-owned). Thin helper, but exposed so CLI and TUI agree +/// on the convention (history/ is always a sibling of portfolio.srf). +pub fn deriveHistoryDir( + allocator: std.mem.Allocator, + portfolio_path: []const u8, +) ![]u8 { + const portfolio_dir = std.fs.path.dirname(portfolio_path) orelse "."; + return std.fs.path.join(allocator, &.{ portfolio_dir, "history" }); +} + +/// Result of `loadTimeline` — bundles the raw snapshot collection and +/// the derived timeline series so callers can reach either without +/// re-parsing. +/// +/// `series.points` is sorted ascending by date; `loaded.snapshots` is +/// in filesystem enumeration order. Both are kept alive together — +/// `series.points` references dates that live inside `loaded`'s +/// snapshot rows, and the callers may want `loaded.snapshots` directly +/// for non-timeline uses (e.g. rollup building). +pub const LoadedTimeline = struct { + loaded: LoadedHistory, + series: timeline.TimelineSeries, + /// Directory we loaded from, caller-owned. Carried through for + /// callers that want to print diagnostics or locate sibling files + /// (rollup.srf, etc.). + history_dir: []u8, + allocator: std.mem.Allocator, + + pub fn deinit(self: *LoadedTimeline) void { + self.series.deinit(); + self.loaded.deinit(); + self.allocator.free(self.history_dir); + } +}; + +/// End-to-end snapshot timeline loader: derives history/, reads every +/// `*-portfolio.srf` file, and builds the sorted timeline series. The +/// single entry point used by both the CLI `zfin history` command and +/// the TUI history tab — their earlier copies had subtle divergences +/// (different dir-split logic, slightly different empty-state ordering) +/// that a shared helper rules out. +/// +/// Returns `loaded.snapshots.len == 0` on an empty history dir rather +/// than erroring; callers check and produce their own "no snapshots" +/// message. Parse failures on individual files are logged to stderr by +/// `loadHistoryDir` and the offending file is skipped. +pub fn loadTimeline( + allocator: std.mem.Allocator, + portfolio_path: []const u8, +) !LoadedTimeline { + const history_dir = try deriveHistoryDir(allocator, portfolio_path); + errdefer allocator.free(history_dir); + + var loaded = try loadHistoryDir(allocator, history_dir); + errdefer loaded.deinit(); + + const series = try timeline.buildSeries(allocator, loaded.snapshots); + + return .{ + .loaded = loaded, + .series = series, + .history_dir = history_dir, + .allocator = allocator, + }; +} + // ── Tests ──────────────────────────────────────────────────── const testing = std.testing; diff --git a/src/tui.zig b/src/tui.zig index 9695b92..b5585ae 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -377,9 +377,11 @@ pub const App = struct { // History tab state history_loaded: bool = false, history_disabled: bool = false, // true when no portfolio path (history requires it) - history_load_result: ?history_io.LoadedHistory = null, - history_series: ?timeline_mod.TimelineSeries = null, - history_metric: timeline_mod.Metric = .net_worth, + history_timeline: ?history_io.LoadedTimeline = null, + // Default to `.liquid` — that's the metric most worth watching + // day-to-day. Illiquid barely changes, net_worth is dominated by + // liquid anyway, so "show me liquid" is the headline view. + history_metric: timeline_mod.Metric = .liquid, // Mouse wheel debounce for cursor-based tabs (portfolio, options). // Terminals often send multiple wheel events per physical tick. diff --git a/src/tui/history_tab.zig b/src/tui/history_tab.zig index b04a46c..991293a 100644 --- a/src/tui/history_tab.zig +++ b/src/tui/history_tab.zig @@ -41,49 +41,27 @@ pub fn loadData(app: *App) void { return; }; - const dir_end = if (std.mem.lastIndexOfScalar(u8, portfolio_path, std.fs.path.sep)) |idx| idx else { - app.setStatus("Cannot derive history/ from portfolio path"); - return; - }; - const portfolio_dir = portfolio_path[0..dir_end]; - - const history_dir = std.fs.path.join(app.allocator, &.{ portfolio_dir, "history" }) catch return; - defer app.allocator.free(history_dir); - - var loaded = history_io.loadHistoryDir(app.allocator, history_dir) catch { + // Shared path with the `zfin history` CLI command — derives + // history/, loads every snapshot, and builds the timeline series. + // If history/ is missing or all files are malformed, we surface a + // status message and leave `app.history_timeline` null; the + // renderer handles the empty case. + app.history_timeline = history_io.loadTimeline(app.allocator, portfolio_path) catch { app.setStatus("Failed to read history/ directory"); return; }; - errdefer loaded.deinit(); - if (loaded.snapshots.len == 0) { - loaded.deinit(); + if (app.history_timeline.?.loaded.snapshots.len == 0) { + freeLoaded(app); app.setStatus("No snapshots in history/ (run: zfin snapshot)"); - return; } - - var series = timeline.buildSeries(app.allocator, loaded.snapshots) catch { - loaded.deinit(); - app.setStatus("Failed to build timeline series"); - return; - }; - - // Commit - app.history_load_result = loaded; - app.history_series = series; - _ = &series; // keep mutable reference (not needed once committed) } -/// Free both the snapshot bytes and the timeline series. Caller must -/// null out the fields after this if they want to allow re-loading. +/// Release the loaded timeline (if any). pub fn freeLoaded(app: *App) void { - if (app.history_series) |s| { - s.deinit(); - app.history_series = null; - } - if (app.history_load_result) |*lh| { - lh.deinit(); - app.history_load_result = null; + if (app.history_timeline) |*tl| { + tl.deinit(); + app.history_timeline = null; } } @@ -99,7 +77,8 @@ pub fn cycleMetric(app: *App) void { // ── Rendering ───────────────────────────────────────────────── pub fn buildStyledLines(app: *App, arena: std.mem.Allocator) ![]const StyledLine { - return renderHistoryLines(arena, app.theme, app.history_series, app.history_metric); + const series: ?timeline.TimelineSeries = if (app.history_timeline) |tl| tl.series else null; + return renderHistoryLines(arena, app.theme, series, app.history_metric); } /// Pure renderer — no App dependency. Builds the styled lines from a @@ -177,13 +156,17 @@ pub fn renderHistoryLines( .style = th.mutedStyle(), }); - // Show up to max_table_rows most recent, but preserve oldest-first - // ordering for deltas to accumulate intuitively. + // Show up to max_table_rows most recent. Render newest-first so + // the latest snapshot sits directly under the column headers, + // matching typical "recent items" lists and saving the eye a scroll. const start = if (points.len > max_table_rows) points.len - max_table_rows else 0; const first = points[0]; // deltas still measured from series start - for (points[start..]) |p| { - const text = try fmtTableRow(arena, p, first); + const window = points[start..]; + var i: usize = window.len; + while (i > 0) { + i -= 1; + const text = try fmtTableRow(arena, window[i], first); try lines.append(arena, .{ .text = text, .style = th.contentStyle() }); }