diff --git a/AGENTS.md b/AGENTS.md index 28754cd..418582b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -752,6 +752,31 @@ will fail to catch real bugs later. 3. Add the dispatch branch in `main.zig`'s command matching chain 4. Update the `usage` string in `main.zig` +### Loading the portfolio (live mode) + +For commands that operate on the live portfolio, **always use +`cli.loadPortfolio(ctx, as_of)`**. It resolves `-p` patterns +through `framework.resolvePatterns` and union-merges every +matching `portfolio_*.srf` file in `ZFIN_HOME` (or cwd), so the +CLI sees the same merged view as the TUI. + +Do not introduce a new "load just the first resolved file" +helper. There used to be a `loadPortfolioFromFile(io, alloc, +path, as_of)` convenience for exactly that — it was deleted +because every production caller had the same bug: a user with +`portfolio.srf` plus sibling `portfolio_NNNN.srf` files saw +silently-different totals from the CLI vs the TUI. The +union-merge contract is now enforced by the absence of any +single-file alternative; keep it that way. + +When you need historical (snapshot) data instead of the live +portfolio, that's a different code path entirely: +`history.loadSnapshotAt(io, alloc, hist_dir, date)` reads a +single `*-portfolio.srf` snapshot file produced by `zfin +snapshot`, and that is the correct single-file load. The +distinction: snapshot files are immutable historical records, +not the user's currently-edited portfolio. + ### Adding a new provider 1. Create `src/providers/newprovider.zig` following the existing struct pattern diff --git a/src/commands/common.zig b/src/commands/common.zig index deb7062..26a0a7f 100644 --- a/src/commands/common.zig +++ b/src/commands/common.zig @@ -368,7 +368,6 @@ pub const LoadedPortfolio = portfolio_loader.LoadedPortfolio; pub const PortfolioData = portfolio_loader.PortfolioData; pub const loadPortfolioFromConfig = portfolio_loader.loadPortfolioFromConfig; pub const loadPortfolioFromPaths = portfolio_loader.loadPortfolioFromPaths; -pub const loadPortfolioFromFile = portfolio_loader.loadPortfolioFromFile; pub const buildPortfolioData = portfolio_loader.buildPortfolioData; /// Resolve `-p`/`--portfolio` patterns through `ctx`, then load the @@ -1051,13 +1050,14 @@ test "fmtAsOfParseError: no trailing newline" { // ── loadPortfolio / buildPortfolioData tests ───────────────── -test "loadPortfolioFromFile: missing file returns null" { +test "loadPortfolioFromPaths: missing file returns null" { const io = std.testing.io; - const result = loadPortfolioFromFile(io, std.testing.allocator, "/nonexistent/portfolio-never-exists.srf", zfin.Date.fromYmd(2026, 5, 8)); + const paths = [_][]const u8{"/nonexistent/portfolio-never-exists.srf"}; + const result = loadPortfolioFromPaths(io, std.testing.allocator, &paths, zfin.Date.fromYmd(2026, 5, 8)); try std.testing.expect(result == null); } -test "loadPortfolioFromFile: malformed file returns null" { +test "loadPortfolioFromPaths: malformed file returns null" { const io = std.testing.io; var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -1069,11 +1069,12 @@ test "loadPortfolioFromFile: malformed file returns null" { const path = try std.fs.path.join(std.testing.allocator, &.{ path_buf[0..dir_len], "bad.srf" }); defer std.testing.allocator.free(path); - const result = loadPortfolioFromFile(io, std.testing.allocator, path, zfin.Date.fromYmd(2026, 5, 8)); + const paths = [_][]const u8{path}; + const result = loadPortfolioFromPaths(io, std.testing.allocator, &paths, zfin.Date.fromYmd(2026, 5, 8)); try std.testing.expect(result == null); } -test "loadPortfolioFromFile: happy path returns LoadedPortfolio with positions and syms" { +test "loadPortfolioFromPaths: single-file happy path returns LoadedPortfolio with positions and syms" { const io = std.testing.io; var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -1091,7 +1092,8 @@ test "loadPortfolioFromFile: happy path returns LoadedPortfolio with positions a const path = try std.fs.path.join(std.testing.allocator, &.{ path_buf[0..dir_len], "portfolio.srf" }); defer std.testing.allocator.free(path); - var loaded = loadPortfolioFromFile(io, std.testing.allocator, path, zfin.Date.fromYmd(2026, 5, 8)) orelse return error.TestUnexpectedResult; + const paths = [_][]const u8{path}; + var loaded = loadPortfolioFromPaths(io, std.testing.allocator, &paths, zfin.Date.fromYmd(2026, 5, 8)) orelse return error.TestUnexpectedResult; defer loaded.deinit(std.testing.allocator); try std.testing.expectEqual(@as(usize, 2), loaded.portfolio.lots.len); @@ -1099,7 +1101,7 @@ test "loadPortfolioFromFile: happy path returns LoadedPortfolio with positions a try std.testing.expectEqual(@as(usize, 2), loaded.syms.len); } -test "loadPortfolioFromFile: today value flows through to position computation" { +test "loadPortfolioFromPaths: today value flows through to position computation" { const io = std.testing.io; var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -1119,15 +1121,16 @@ test "loadPortfolioFromFile: today value flows through to position computation" const path = try std.fs.path.join(std.testing.allocator, &.{ path_buf[0..dir_len], "portfolio.srf" }); defer std.testing.allocator.free(path); + const paths = [_][]const u8{path}; // today before open_date → position exists but no open shares - var loaded_before = loadPortfolioFromFile(io, std.testing.allocator, path, zfin.Date.fromYmd(2024, 1, 1)) orelse return error.TestUnexpectedResult; + var loaded_before = loadPortfolioFromPaths(io, std.testing.allocator, &paths, zfin.Date.fromYmd(2024, 1, 1)) orelse return error.TestUnexpectedResult; defer loaded_before.deinit(std.testing.allocator); try std.testing.expectEqual(@as(usize, 1), loaded_before.positions.len); try std.testing.expectApproxEqAbs(@as(f64, 0), loaded_before.positions[0].shares, 0.01); try std.testing.expectEqual(@as(u32, 0), loaded_before.positions[0].open_lots); // today after open_date → 100 shares open - var loaded_after = loadPortfolioFromFile(io, std.testing.allocator, path, zfin.Date.fromYmd(2025, 1, 1)) orelse return error.TestUnexpectedResult; + var loaded_after = loadPortfolioFromPaths(io, std.testing.allocator, &paths, zfin.Date.fromYmd(2025, 1, 1)) orelse return error.TestUnexpectedResult; defer loaded_after.deinit(std.testing.allocator); try std.testing.expectEqual(@as(usize, 1), loaded_after.positions.len); try std.testing.expectApproxEqAbs(@as(f64, 100), loaded_after.positions[0].shares, 0.01); diff --git a/src/commands/compare.zig b/src/commands/compare.zig index c49cc1f..9043d05 100644 --- a/src/commands/compare.zig +++ b/src/commands/compare.zig @@ -390,12 +390,18 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { var then_side = try compare_core.loadSnapshotSide(io, allocator, hist_dir, then_date); defer then_side.deinit(allocator); + // Pre-load live-portfolio data once when the "now" side is live, + // shared between the projections key-metrics block and the + // LiveSide aggregation below. + var live_data: ?projections.LiveData = null; + defer if (live_data) |*ld| ld.deinit(allocator); + if (now_is_live) { + live_data = try projections.loadLiveData(ctx, as_of, color); + } + // Projections: only computed when --projections/-p flag is set. // Uses the SNAPPED dates (not requested) because projections are // snapshot-based — they need actual files on disk to load. - // When `now_is_live`, pass null for the "now" side so projections - // uses the live portfolio. When two-date mode, pass the snapped - // now_date. var projections_result: ?projections.KeyComparisonResult = null; defer if (projections_result) |r| r.cleanup(); var projections_block: ?ProjectionsBlock = null; @@ -412,7 +418,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { .vs_date = then_date, .now_date = proj_now_date, .now_from_snapshot = !now_is_live, - .refresh = ctx.globals.refresh_policy, + .live_for_now = if (live_data) |*ld| ld else null, }, ) catch |err| blk: { // Projections computation failed — fall back to compare @@ -442,7 +448,17 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { .{ .date_at_or_before = now_date_requested }; if (now_is_live) { - var now_live = try LiveSide.load(io, allocator, svc, portfolio_path, as_of, color, ctx.globals.refresh_policy); + // Borrow the pre-loaded live_data to build the LiveSide + // map. When `live_data` is null (loadLiveData returned + // null due to a load failure), fall back to a fresh + // owned load so the user still gets the Liquid / + // per-symbol output. + var now_live: LiveSide = if (live_data) |ld| blk: { + break :blk LiveSide.fromLiveData(allocator, as_of, ld) catch { + cli.stderrPrint(io, "Portfolio is empty.\n"); + return; + }; + } else try LiveSide.loadOwned(ctx, as_of, color); defer now_live.deinit(allocator); // Attribution uses the resolved CommitSpecs so --commit-* @@ -580,74 +596,71 @@ fn renderFromParts( /// Not used by the TUI — the TUI uses its already-loaded portfolio /// state directly and calls `compare_core.aggregateLiveStocks` inline. const LiveSide = struct { - loaded: cli.LoadedPortfolio, - pf_data: cli.PortfolioData, - prices: std.StringHashMap(f64), + /// Underlying live-portfolio data. Populated either by loading + /// fresh (when `LiveSide.loadOwned` is used) or by borrowing + /// from a caller-supplied `LiveData` (`LiveSide.fromLiveData`). + /// `owned` flags ownership: when true, `deinit` frees `live`; + /// when false, the caller frees the borrowed source. + live: projections.LiveData, + owned: bool, map: view.HoldingMap, liquid: f64, - fn load( - io: std.Io, + /// Build a LiveSide that *borrows* an already-loaded `LiveData`. + /// Used in `compare`'s `with_projections && now_is_live` branch + /// where projections has already loaded the live portfolio for + /// its key-metrics block — re-loading would be wasted work and + /// would re-fetch prices unnecessarily. + fn fromLiveData( allocator: std.mem.Allocator, - svc: *zfin.DataService, - portfolio_path: []const u8, as_of: Date, - color: bool, - refresh: framework.RefreshPolicy, + src: projections.LiveData, ) !LiveSide { - var loaded_pf = cli.loadPortfolioFromFile(io, allocator, portfolio_path, as_of) orelse return error.PortfolioLoadFailed; - errdefer loaded_pf.deinit(allocator); - - if (loaded_pf.portfolio.lots.len == 0) { - cli.stderrPrint(io, "Portfolio is empty.\n"); - return error.PortfolioLoadFailed; + if (src.loaded.portfolio.lots.len == 0) { + return error.PortfolioEmpty; } - - var prices = std.StringHashMap(f64).init(allocator); - errdefer prices.deinit(); - - if (loaded_pf.syms.len > 0) { - var load_result = cli.loadPortfolioPrices(io, svc, loaded_pf.syms, &.{}, refresh, color); - defer load_result.deinit(); - var it = load_result.prices.iterator(); - while (it.next()) |entry| try prices.put(entry.key_ptr.*, entry.value_ptr.*); - } - - var pf_data = cli.buildPortfolioData( - allocator, - loaded_pf.portfolio, - loaded_pf.positions, - loaded_pf.syms, - &prices, - svc, - as_of, - ) catch |err| switch (err) { - error.NoAllocations, error.SummaryFailed => { - cli.stderrPrint(io, "Error computing portfolio summary.\n"); - return error.PortfolioLoadFailed; - }, - else => return err, - }; - errdefer pf_data.deinit(allocator); - var map: view.HoldingMap = .init(allocator); errdefer map.deinit(); - try compare_core.aggregateLiveStocks(as_of, &loaded_pf.portfolio, &prices, &map); - + var prices = src.prices; + try compare_core.aggregateLiveStocks(as_of, &src.loaded.portfolio, &prices, &map); return .{ - .loaded = loaded_pf, - .pf_data = pf_data, - .prices = prices, + .live = src, + .owned = false, .map = map, - .liquid = pf_data.summary.total_value, + .liquid = src.pf_data.summary.total_value, + }; + } + + /// Load a LiveSide standalone (compare without --projections). + /// Goes through `cli.loadPortfolio`, which honors the multi-file + /// union-merge path — matching what the TUI sees. + fn loadOwned( + ctx: *framework.RunCtx, + as_of: Date, + color: bool, + ) !LiveSide { + const allocator = ctx.allocator; + const live = (try projections.loadLiveData(ctx, as_of, color)) orelse return error.PortfolioLoadFailed; + var owned = live; + errdefer owned.deinit(allocator); + if (owned.loaded.portfolio.lots.len == 0) { + cli.stderrPrint(ctx.io, "Portfolio is empty.\n"); + return error.PortfolioLoadFailed; + } + var map: view.HoldingMap = .init(allocator); + errdefer map.deinit(); + try compare_core.aggregateLiveStocks(as_of, &owned.loaded.portfolio, &owned.prices, &map); + return .{ + .live = owned, + .owned = true, + .map = map, + .liquid = owned.pf_data.summary.total_value, }; } fn deinit(self: *LiveSide, allocator: std.mem.Allocator) void { self.map.deinit(); - self.prices.deinit(); - self.pf_data.deinit(allocator); - self.loaded.deinit(allocator); + if (self.owned) self.live.deinit(allocator); } }; diff --git a/src/commands/projections.zig b/src/commands/projections.zig index cfd977b..4550690 100644 --- a/src/commands/projections.zig +++ b/src/commands/projections.zig @@ -244,29 +244,42 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { .convergence => try runConvergence(io, allocator, file_path, color, out), .return_backtest => |args| try runReturnBacktest(io, allocator, file_path, args.real, color, out), .compare => |args| { - const svc = ctx.svc orelse return error.MissingDataService; + _ = ctx.svc orelse return error.MissingDataService; + // Pre-load live data when the "now" side is live so it + // can be shared with `computeKeyComparison`. The + // snapshot-vs-snapshot path doesn't need it. + const now_is_live = args.as_of == null; + var live: ?LiveData = null; + defer if (live) |*l| l.deinit(allocator); + if (now_is_live) { + live = try loadLiveData(ctx, today, color); + } try runCompare( - io, - allocator, - svc, + ctx, file_path, .{ .events_enabled = args.events_enabled, .vs_date = args.vs_date, .now_date = args.as_of orelse today, .now_from_snapshot = args.as_of != null, - .refresh = ctx.globals.refresh_policy, + .live_for_now = if (live) |*l| l else null, }, - color, - out, ); }, .bands => |args| { - const svc = ctx.svc orelse return error.MissingDataService; + _ = ctx.svc orelse return error.MissingDataService; + // Pre-load live data unconditionally for the bands view. + // The live (no `--as-of`) path needs it for the simulation; + // the imported-only as-of branch (when `--as-of ` + // resolves to an imported value rather than a snapshot) + // needs today's allocations to scale to the imported + // total. Snapshot-only as-of paths ignore it. + var live = try loadLiveData(ctx, today, color); + defer if (live) |*l| l.deinit(allocator); try runBands( io, allocator, - svc, + ctx.svc.?, file_path, .{ .events_enabled = args.events_enabled, @@ -274,8 +287,8 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { .from_snapshot = args.as_of != null, .today = today, .overlay_actuals = args.overlay_actuals, - .refresh = ctx.globals.refresh_policy, .export_chart = args.export_chart, + .live = if (live) |*l| l else null, }, color, out, @@ -312,6 +325,80 @@ const AsOfResolution = struct { /// today as `as_of`. /// - `true`: historical mode. Load the snapshot at-or-before /// `as_of` from the history dir. +/// Pre-loaded live-portfolio data used by `runBands` and +/// `computeKeyComparison`. The caller (typically `run()`) loads +/// this via `cli.loadPortfolio(ctx, today)` so the multi-file +/// union-merge path is always taken — matching what the TUI sees +/// and what every other CLI command sees. +/// +/// Loading lives in the caller (not in `runBands` / +/// `computeKeyComparison`) so the helpers can't accidentally take +/// a single-file path. They consume what the caller passes in; +/// the union-merge contract is enforced at one site. +/// +/// `prices` is retained alongside the summary so callers that +/// need per-symbol price lookups (e.g. `compare`'s LiveSide via +/// `compare_core.aggregateLiveStocks`) can borrow without +/// re-fetching. +/// +/// Caller owns the fields and must call `deinit` after the helper +/// returns. +pub const LiveData = struct { + loaded: cli.LoadedPortfolio, + pf_data: cli.PortfolioData, + prices: std.StringHashMap(f64), + + pub fn deinit(self: *LiveData, allocator: std.mem.Allocator) void { + self.prices.deinit(); + self.pf_data.deinit(allocator); + self.loaded.deinit(allocator); + } +}; + +/// Load the live portfolio (multi-file union-merge), fetch prices +/// per the resolved refresh policy, and build the +/// summary/candle-map bundle. Mirrors what every other CLI command +/// does via `cli.loadPortfolio` + `cli.loadPortfolioPrices` + +/// `cli.buildPortfolioData`. +/// +/// Returns `null` when the portfolio fails to load (the loader has +/// already printed a stderr message). On `error.NoAllocations` / +/// `error.SummaryFailed` the helper writes a friendly stderr line +/// and returns `null` so the caller can fall through cleanly. +pub fn loadLiveData( + ctx: *framework.RunCtx, + today: Date, + color: bool, +) !?LiveData { + const io = ctx.io; + const allocator = ctx.allocator; + const svc = ctx.svc orelse return error.MissingDataService; + + var loaded = cli.loadPortfolio(ctx, today) orelse return null; + errdefer loaded.deinit(allocator); + + var prices = std.StringHashMap(f64).init(allocator); + errdefer prices.deinit(); + { + var load_result = cli.loadPortfolioPrices(io, svc, loaded.syms, &.{}, ctx.globals.refresh_policy, color); + defer load_result.deinit(); + var it = load_result.prices.iterator(); + while (it.next()) |entry| { + try prices.put(entry.key_ptr.*, entry.value_ptr.*); + } + } + + const pf_data = cli.buildPortfolioData(allocator, loaded.portfolio, loaded.positions, loaded.syms, &prices, svc, today) catch |err| switch (err) { + error.NoAllocations, error.SummaryFailed => { + cli.stderrPrint(io, "Error computing portfolio summary.\n"); + return null; + }, + else => return err, + }; + + return .{ .loaded = loaded, .pf_data = pf_data, .prices = prices }; +} + /// Per-call configuration for `runBands`. Bundled because the /// call already had nine context-plus-config parameters and adding /// `refresh` would push it past the readable-positional threshold. @@ -336,13 +423,18 @@ pub const BandsOptions = struct { /// Whether to overlay an actual-history series on the /// projection percentile bands. overlay_actuals: bool, - /// Refresh policy threaded through the live "now" price - /// load. Has no effect when `from_snapshot = true`. - refresh: framework.RefreshPolicy = .auto, /// When set, render the percentile-band chart (with optional /// overlay) to a PNG at this path and exit before any text /// output renders. See `chart_export.exportProjectionChart`. export_chart: ?[]const u8 = null, + /// Pre-loaded live-portfolio data. Required when: + /// - `from_snapshot == false` (the live-portfolio path), OR + /// - `from_snapshot == true` AND the as-of resolution lands + /// on an `imported_values.srf` row (the imported-only + /// branch needs today's allocations to scale). + /// Snapshot-only as-of paths ignore this field. See + /// `LiveData` for the rationale. + live: ?*const LiveData = null, }; pub fn runBands( @@ -379,13 +471,6 @@ pub fn runBands( // defer runs at the end of `run`. var snap_bundle: ?history.LoadedSnapshot = null; defer if (snap_bundle) |*s| s.deinit(allocator); - // Live-portfolio bundle. Loaded for the live (no-as-of) path - // AND for the imported-only as-of path (which uses today's - // composition scaled to the imported liquid total). - var live_loaded: ?cli.LoadedPortfolio = null; - defer if (live_loaded) |*l| l.deinit(allocator); - var live_pf_data: ?cli.PortfolioData = null; - defer if (live_pf_data) |*p| p.deinit(allocator); if (opts.from_snapshot) { resolution = resolveAsOfSnapshot(io, va, file_path, opts.as_of) catch |err| switch (err) { @@ -408,45 +493,20 @@ pub fn runBands( ); } else { // Imported-only as-of: need today's portfolio composition - // (allocations + cash/CD totals) to scale to the - // imported liquid value. - // - // Today's `as_of` parameter is being repurposed here as - // the historical reference date for the simulation; for - // the composition we ALWAYS want today's mix (the only - // composition we know). Pass `today` for the cash/CD - // computation. - live_loaded = cli.loadPortfolioFromFile(io, allocator, file_path, opts.today) orelse return; - const lp = &live_loaded.?; - - // Route through the shared loader so `--refresh-data` - // propagates here. Pre-bundle this was a silent - // cache-only loop; behavior change is intentional. - var prices = std.StringHashMap(f64).init(allocator); - defer prices.deinit(); - { - var load_result = cli.loadPortfolioPrices(io, svc, lp.syms, &.{}, opts.refresh, color); - defer load_result.deinit(); - var it = load_result.prices.iterator(); - while (it.next()) |entry| { - try prices.put(entry.key_ptr.*, entry.value_ptr.*); - } - } - - live_pf_data = cli.buildPortfolioData(allocator, lp.portfolio, lp.positions, lp.syms, &prices, svc, opts.today) catch |err| switch (err) { - error.NoAllocations, error.SummaryFailed => { - cli.stderrPrint(io, "Error computing portfolio summary for imported-only as-of.\n"); - return; - }, - else => return err, + // (allocations + cash/CD totals) to scale to the imported + // liquid value. Caller supplies it in `opts.live`. + const live = opts.live orelse { + cli.stderrPrint(io, "Error: imported-only as-of resolution requires live portfolio data; pass `opts.live`.\n"); + return; }; + const lp = &live.loaded; ctx = try view.loadProjectionContextFromImported( io, va, portfolio_dir, - live_pf_data.?.summary.allocations, - live_pf_data.?.summary.total_value, + live.pf_data.summary.allocations, + live.pf_data.summary.total_value, lp.portfolio.totalCash(opts.today), lp.portfolio.totalCdFaceValue(opts.today), resolution.?.liquid, @@ -456,37 +516,19 @@ pub fn runBands( ); } } else { - live_loaded = cli.loadPortfolioFromFile(io, allocator, file_path, opts.as_of) orelse return; - const lp = &live_loaded.?; - - // Route through the shared loader so `--refresh-data` - // propagates here. Pre-bundle this was a silent - // cache-only loop; behavior change is intentional. - var prices = std.StringHashMap(f64).init(allocator); - defer prices.deinit(); - { - var load_result = cli.loadPortfolioPrices(io, svc, lp.syms, &.{}, opts.refresh, color); - defer load_result.deinit(); - var it = load_result.prices.iterator(); - while (it.next()) |entry| { - try prices.put(entry.key_ptr.*, entry.value_ptr.*); - } - } - - live_pf_data = cli.buildPortfolioData(allocator, lp.portfolio, lp.positions, lp.syms, &prices, svc, opts.as_of) catch |err| switch (err) { - error.NoAllocations, error.SummaryFailed => { - cli.stderrPrint(io, "Error computing portfolio summary.\n"); - return; - }, - else => return err, + // Live path. Caller supplies pre-loaded data in `opts.live`. + const live = opts.live orelse { + cli.stderrPrint(io, "Error: live projections require pre-loaded `opts.live` data.\n"); + return; }; + const lp = &live.loaded; ctx = try view.loadProjectionContext( io, va, portfolio_dir, - live_pf_data.?.summary.allocations, - live_pf_data.?.summary.total_value, + live.pf_data.summary.allocations, + live.pf_data.summary.total_value, lp.portfolio.totalCash(opts.as_of), lp.portfolio.totalCdFaceValue(opts.as_of), svc, @@ -874,14 +916,16 @@ fn loadAsOfContext( /// For the full benchmark table / SWR grid / percentile bands, run /// `zfin projections` and `zfin projections --as-of ` separately. pub fn runCompare( - io: std.Io, - allocator: std.mem.Allocator, - svc: *zfin.DataService, + ctx: *framework.RunCtx, file_path: []const u8, opts: KeyComparisonOptions, - color: bool, - out: *std.Io.Writer, ) !void { + const io = ctx.io; + const allocator = ctx.allocator; + const out = ctx.out; + const color = ctx.color; + const svc = ctx.svc orelse return error.MissingDataService; + var arena_state = std.heap.ArenaAllocator.init(allocator); defer arena_state.deinit(); const va = arena_state.allocator(); @@ -1084,10 +1128,8 @@ pub const KeyComparisonResult = struct { /// Per-call configuration for `computeKeyComparison`. Bundled into /// a struct because the call already had eight context-plus-config -/// parameters and adding `refresh` would push it to ten — past the -/// point where positional args are readable. Required fields have -/// no defaults; optional knobs (currently just `refresh`) carry -/// sensible defaults so most callers can leave them out. +/// parameters and adding more knobs would push it past the readable +/// positional threshold. pub const KeyComparisonOptions = struct { /// Whether simulated lifecycle events (RMDs, lump-sum /// withdrawals, Social Security) are baked into the @@ -1099,13 +1141,17 @@ pub const KeyComparisonOptions = struct { /// controlled by `now_from_snapshot`. now_date: Date, /// When true, both sides resolve from snapshots. When - /// false, the "now" side loads the live portfolio and - /// fetches current prices. + /// false, the "now" side uses the live portfolio supplied + /// via `live_for_now`. now_from_snapshot: bool, - /// Refresh policy threaded through the live "now" price - /// load. Has no effect when `now_from_snapshot = true` - /// (snapshots don't fetch prices). - refresh: framework.RefreshPolicy = .auto, + /// Pre-loaded live-portfolio data for the "now" side. + /// REQUIRED when `now_from_snapshot == false`; ignored + /// otherwise. The caller (typically `run` or + /// `commands/compare.zig`'s `run`) loads this via + /// `loadLiveData(ctx, ...)` so the multi-file union-merge + /// path is always taken. See `LiveData`'s doc-comment for + /// why the load lives in the caller and not here. + live_for_now: ?*const LiveData = null, }; /// Compute the "then" vs "now" key metrics for `--vs` and the @@ -1194,49 +1240,22 @@ pub fn computeKeyComparison( }; } - // Live "now" side — mirrors `run()`'s live path. - var loaded = cli.loadPortfolioFromFile(io, allocator, file_path, opts.now_date) orelse { + // Live "now" side. The caller pre-loads via `loadLiveData` and + // passes the result in `opts.live_for_now`. + const live = opts.live_for_now orelse { then_snap.deinit(allocator); + cli.stderrPrint(io, "Error: live `now` side requires pre-loaded `opts.live_for_now`.\n"); return error.PortfolioLoadFailed; }; - defer loaded.deinit(allocator); - - // Route through the shared loader so `--refresh-data` propagates - // here too. Pre-bundle this was a silent cache-only loop, which - // diverged from every other multi-symbol command. Watchlist syms - // aren't relevant for projections so we pass an empty slice. - var prices = std.StringHashMap(f64).init(allocator); - defer prices.deinit(); - { - var load_result = cli.loadPortfolioPrices(io, svc, loaded.syms, &.{}, opts.refresh, false); - defer load_result.deinit(); - var it = load_result.prices.iterator(); - while (it.next()) |entry| { - try prices.put(entry.key_ptr.*, entry.value_ptr.*); - } - } - - var pf_data = cli.buildPortfolioData(allocator, loaded.portfolio, loaded.positions, loaded.syms, &prices, svc, opts.now_date) catch |err| switch (err) { - error.NoAllocations, error.SummaryFailed => { - then_snap.deinit(allocator); - cli.stderrPrint(io, "Error computing portfolio summary.\n"); - return error.PortfolioLoadFailed; - }, - else => { - then_snap.deinit(allocator); - return err; - }, - }; - defer pf_data.deinit(allocator); const now_ctx = try view.loadProjectionContext( io, va, portfolio_dir, - pf_data.summary.allocations, - pf_data.summary.total_value, - loaded.portfolio.totalCash(opts.now_date), - loaded.portfolio.totalCdFaceValue(opts.now_date), + live.pf_data.summary.allocations, + live.pf_data.summary.total_value, + live.loaded.portfolio.totalCash(opts.now_date), + live.loaded.portfolio.totalCdFaceValue(opts.now_date), svc, opts.events_enabled, opts.now_date, diff --git a/src/portfolio_loader.zig b/src/portfolio_loader.zig index 36ff26d..0ca2d4e 100644 --- a/src/portfolio_loader.zig +++ b/src/portfolio_loader.zig @@ -36,10 +36,6 @@ //! the TUI's reload-button path (re-uses the original resolved //! path slice without re-globbing). //! -//! - `loadPortfolioFromFile(io, alloc, path, as_of)` — single -//! file. Used by CLI `compare` / `projections` for snapshot -//! reads where a specific historical file is loaded. -//! //! - `PortfolioData` + `buildPortfolioData(...)` — second-stage //! pipeline: turn a `LoadedPortfolio` (or its parts) plus a //! `prices` map into a `PortfolioSummary` with allocations, @@ -328,14 +324,6 @@ fn loadFromPaths( }; } -/// Convenience for tests + single-file CLI paths (compare, -/// projections snapshot reads). Wraps `loadPortfolioFromPaths` -/// with a one-element slice. -pub fn loadPortfolioFromFile(io: std.Io, allocator: std.mem.Allocator, file_path: []const u8, as_of: zfin.Date) ?LoadedPortfolio { - const paths = [_][]const u8{file_path}; - return loadPortfolioFromPaths(io, allocator, &paths, as_of); -} - // ── Portfolio data pipeline ────────────────────────────────── /// Result of the shared portfolio data pipeline. Caller must call deinit().