From fef126471f44dc09077be6cfe9185e6d6253e2f0 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sat, 2 May 2026 11:02:06 -0700 Subject: [PATCH] clean up contributions command --- src/analytics/analysis.zig | 44 ++ src/commands/common.zig | 107 +++++ src/commands/compare.zig | 389 ++++++++++++---- src/commands/contributions.zig | 820 +++++++++++++++++++++++++++++++-- src/commands/history.zig | 10 +- src/commands/projections.zig | 39 +- src/commands/snapshot.zig | 7 +- src/git.zig | 14 +- src/views/compare.zig | 103 +++++ 9 files changed, 1381 insertions(+), 152 deletions(-) diff --git a/src/analytics/analysis.zig b/src/analytics/analysis.zig index 174df37..510b525 100644 --- a/src/analytics/analysis.zig +++ b/src/analytics/analysis.zig @@ -42,6 +42,18 @@ pub const AccountTaxEntry = struct { institution: ?[]const u8 = null, account_number: ?[]const u8 = null, update_cadence: UpdateCadence = .weekly, + /// When true, raw cash-balance changes (`cash_delta` in the + /// contributions diff) on this account roll up into the + /// attribution total as real contributions. + /// + /// Defaults to false because most cash accounts generate + /// `cash_delta` entries from internal movement — interest posting, + /// dividend credit, CD coupon, settlement sweeps — that would + /// inflate the attribution number if counted. Set to true only + /// for accounts whose cash movement is dominated by external + /// contributions (payroll ESPP accrual, direct 401k cash + /// deposits). See TODO.md for the design history. + cash_is_contribution: bool = false, }; /// Update cadence for manual account maintenance. Parsed from accounts.srf. @@ -120,6 +132,18 @@ pub const AccountMap = struct { if (count == 0) return &.{}; return self.entries; } + + /// Is cash-balance movement on `account` treated as a real + /// contribution (vs. internal noise) for the attribution total? + /// Defaults to false when the account isn't in the map. + pub fn cashIsContribution(self: AccountMap, account: []const u8) bool { + for (self.entries) |e| { + if (std.mem.eql(u8, e.account, account)) { + return e.cash_is_contribution; + } + } + return false; + } }; /// Parse an accounts.srf file into an AccountMap. @@ -147,6 +171,7 @@ pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !Accoun .institution = if (entry.institution) |s| try allocator.dupe(u8, s) else null, .account_number = if (entry.account_number) |s| try allocator.dupe(u8, s) else null, .update_cadence = entry.update_cadence, + .cash_is_contribution = entry.cash_is_contribution, }); } @@ -377,6 +402,25 @@ test "parseAccountsFile" { try std.testing.expectEqualStrings("Unknown", am.taxTypeFor("Nonexistent")); } +test "parseAccountsFile: cash_is_contribution default false, opt-in true" { + const data = + \\#!srfv1 + \\account::Kelly ESPP,tax_type::taxable,cash_is_contribution:bool:true + \\account::Joint cash,tax_type::taxable + ; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + + try std.testing.expectEqual(@as(usize, 2), am.entries.len); + // Opted-in account + try std.testing.expect(am.cashIsContribution("Kelly ESPP")); + // Default-off account + try std.testing.expect(!am.cashIsContribution("Joint cash")); + // Unknown account defaults to false + try std.testing.expect(!am.cashIsContribution("Nonexistent")); +} + test "TaxType.label" { try std.testing.expectEqualStrings("Taxable", TaxType.taxable.label()); try std.testing.expectEqualStrings("Roth (Post-Tax)", TaxType.roth.label()); diff --git a/src/commands/common.zig b/src/commands/common.zig index 88e562f..f3a94d9 100644 --- a/src/commands/common.zig +++ b/src/commands/common.zig @@ -1,6 +1,7 @@ const std = @import("std"); const zfin = @import("../root.zig"); const srf = @import("srf"); +const history = @import("../history.zig"); pub const fmt = @import("../format.zig"); // ── Default CLI colors (match TUI default Monokai theme) ───── @@ -521,6 +522,112 @@ pub fn fmtAsOfParseError(buf: []u8, input: []const u8, err: AsOfParseError) []co }; } +/// Parse a user-facing date argument that must resolve to a concrete +/// absolute date — no "live"/"now"/empty. Accepts the same grammar +/// as `parseAsOfDate` (`YYYY-MM-DD` or relative shortcuts like `1W`, +/// `1M`, `1Q`, `1Y`, case-insensitive) minus the null-producing +/// inputs. Used by commands where a date-argument bound to a +/// specific date makes sense but "live" doesn't — e.g. `compare`'s +/// positional args, `history --since`/`--until`, `snapshot --as-of`. +/// +/// `today` is injected for test determinism. Production callers pass +/// `fmt.todayDate()`. +pub const RequiredDateError = AsOfParseError || error{LiveNotAllowed}; + +pub fn parseRequiredDate(input: []const u8, today: zfin.Date) RequiredDateError!zfin.Date { + const parsed = try parseAsOfDate(input, today); + return parsed orelse error.LiveNotAllowed; +} + +/// Convenience pattern: parse a required date, print a helpful error +/// to stderr if it fails, and map every failure mode to a single +/// `error.InvalidDate`. Callers get a uniform error, stderr gets a +/// message that tells the user exactly what grammar is accepted +/// including the relative-shortcut syntax. +/// +/// `today` is injected for test determinism. +pub fn parseRequiredDateOrStderr( + input: []const u8, + today: zfin.Date, + arg_label: []const u8, +) error{InvalidDate}!zfin.Date { + return parseRequiredDate(input, today) catch |err| { + var ebuf: [256]u8 = undefined; + const msg = switch (err) { + error.LiveNotAllowed => std.fmt.bufPrint( + &ebuf, + "Error: {s} must be a concrete date, not 'live'/'now'.\n", + .{arg_label}, + ) catch "Error: invalid date\n", + else => |e| blk: { + var inner: [256]u8 = undefined; + const detail = fmtAsOfParseError(&inner, input, e); + break :blk std.fmt.bufPrint( + &ebuf, + "Error: {s}: {s} (expected YYYY-MM-DD or a relative shortcut like 1W/1M/1Q/1Y)\n", + .{ arg_label, detail }, + ) catch "Error: invalid date\n"; + }, + }; + stderrPrint(msg) catch {}; + return error.InvalidDate; + }; +} + +// ── Snapshot resolution (CLI) ──────────────────────────────── + +/// Snap a requested snapshot date to the nearest earlier snapshot +/// that exists in `hist_dir`, printing CLI-friendly stderr messages +/// when resolution fails. +/// +/// Thin wrapper over `history.resolveSnapshotDate` that bundles the +/// "no snapshot at or before X, earliest available is Y" hint that +/// both `projections --as-of` and `compare` surface to the user. +/// Returns the full `ResolvedSnapshot` so callers can distinguish +/// exact vs. inexact matches (compare uses this to print a muted +/// "snapped to …" notice, projections uses `actual != requested` to +/// drive the header). +/// +/// On `error.NoSnapshotAtOrBefore` the stderr messages are emitted +/// and the error is propagated verbatim; callers typically map it to +/// their own command-level error (`error.NoSnapshot`, +/// `error.SnapshotNotFound`, etc.). Other errors propagate without a +/// stderr write — they indicate filesystem-level failures the caller +/// should surface itself. +/// +/// Uses `arena` for the intermediate message strings; pass a +/// short-lived arena. +pub fn resolveSnapshotOrExplain( + arena: std.mem.Allocator, + hist_dir: []const u8, + requested: zfin.Date, +) !history.ResolvedSnapshot { + return history.resolveSnapshotDate(arena, hist_dir, requested) catch |err| switch (err) { + error.NoSnapshotAtOrBefore => { + var req_buf: [10]u8 = undefined; + const req_str = requested.format(&req_buf); + const msg = std.fmt.allocPrint(arena, "No snapshot at or before {s}.\n", .{req_str}) catch "No snapshot at or before the requested date.\n"; + stderrPrint(msg) catch {}; + // Second look at the nearest table for the "later + // available" hint. Cheap (filesystem scan, same dir). + const nearest = history.findNearestSnapshot(hist_dir, requested) catch { + stderrPrint("No snapshots in history/ — run `zfin snapshot` to create one.\n") catch {}; + return err; + }; + if (nearest.later) |later| { + var later_buf: [10]u8 = undefined; + const later_str = later.format(&later_buf); + const later_msg = std.fmt.allocPrint(arena, "Earliest available: {s} (later than requested).\n", .{later_str}) catch "A later snapshot exists but was not used.\n"; + stderrPrint(later_msg) catch {}; + } else { + stderrPrint("No snapshots in history/ — run `zfin snapshot` to create one.\n") catch {}; + } + return err; + }, + else => |e| return e, + }; +} + // ── Watchlist loading ──────────────────────────────────────── /// Load a watchlist SRF file containing symbol records. diff --git a/src/commands/compare.zig b/src/commands/compare.zig index 2bd2009..639e417 100644 --- a/src/commands/compare.zig +++ b/src/commands/compare.zig @@ -54,7 +54,7 @@ const history = @import("../history.zig"); const compare_core = @import("../compare.zig"); const view = @import("../views/compare.zig"); const view_hist = @import("../views/history.zig"); -const contributions_cmd = @import("contributions.zig"); +const contributions = @import("contributions.zig"); pub const Error = error{ UnexpectedArg, @@ -76,10 +76,11 @@ pub fn run( ) !void { // ── Parse args ─────────────────────────────────────────── if (cmd_args.len == 0) { - try cli.stderrPrint("Error: 'compare' requires one or two dates (YYYY-MM-DD).\n"); + try cli.stderrPrint("Error: 'compare' requires one or two dates.\n"); try cli.stderrPrint("Usage:\n"); try cli.stderrPrint(" zfin compare (compare date vs current)\n"); try cli.stderrPrint(" zfin compare (compare two dates)\n"); + try cli.stderrPrint("Dates accept YYYY-MM-DD or relative shortcuts: 1W, 1M, 1Q, 1Y.\n"); return error.MissingDateArg; } if (cmd_args.len > 2) { @@ -87,11 +88,9 @@ pub fn run( return error.UnexpectedArg; } - const date1 = Date.parse(cmd_args[0]) catch { - try cli.stderrPrint("Error: invalid date format: "); - try cli.stderrPrint(cmd_args[0]); - try cli.stderrPrint(" (expected YYYY-MM-DD)\n"); - return error.InvalidDate; + const today = fmt.todayDate(); + const date1 = cli.parseRequiredDateOrStderr(cmd_args[0], today, "date1") catch |err| switch (err) { + error.InvalidDate => return error.InvalidDate, }; // Resolve (then_date, now_date, now_is_live). In single-date mode the @@ -99,12 +98,9 @@ pub fn run( // portfolio). In two-date mode both are snapshots and we swap to // guarantee older → newer. const date2: ?Date = if (cmd_args.len == 2) - Date.parse(cmd_args[1]) catch { - try cli.stderrPrint("Error: invalid date format: "); - try cli.stderrPrint(cmd_args[1]); - try cli.stderrPrint(" (expected YYYY-MM-DD)\n"); - return error.InvalidDate; - } + (cli.parseRequiredDateOrStderr(cmd_args[1], today, "date2") catch |err| switch (err) { + error.InvalidDate => return error.InvalidDate, + }) else null; @@ -115,7 +111,6 @@ pub fn run( // SAFETY: see above. var now_date: Date = undefined; if (now_is_live) { - const today = fmt.todayDate(); if (date1.days == today.days) { try cli.stderrPrint("Error: cannot compare today against today's live portfolio.\n"); return error.SameDate; @@ -142,28 +137,70 @@ pub fn run( const hist_dir = try history.deriveHistoryDir(allocator, portfolio_path); defer allocator.free(hist_dir); + // ── Snap dates to nearest-earlier snapshots ────────────── + // + // The requested date may not correspond to an actual snapshot + // file (e.g. `1W` resolves to a weekday but snapshots only exist + // for business days, or the cron was down that day). Snap to the + // most recent snapshot at-or-before the requested date — same + // semantics as `projections --as-of`. When the snap is inexact, + // overwrite the date used downstream so the rendered header + // reflects what was actually loaded. + // + // IMPORTANT: we keep the *original requested* dates around for + // attribution. The attribution is a git-window query that should + // match what `zfin contributions --since ` would + // produce — if we passed the snapped dates to attribution, the + // git window would widen every time the requested date doesn't + // land on a snapshot, causing compare's attribution to disagree + // with contributions' totals over the same nominal window. + // Snapshot loading uses the snapped dates (so we actually find a + // file on disk); attribution uses the original dates. + var arena_state = std.heap.ArenaAllocator.init(allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const then_date_requested = then_date; + const now_date_requested = now_date; + + const then_resolved = cli.resolveSnapshotOrExplain(arena, hist_dir, then_date) catch return error.SnapshotNotFound; + if (!then_resolved.exact) { + try printSnapNote(color, then_resolved.requested, then_resolved.actual, "then"); + } + then_date = then_resolved.actual; + + if (!now_is_live) { + const now_resolved = cli.resolveSnapshotOrExplain(arena, hist_dir, now_date) catch return error.SnapshotNotFound; + if (!now_resolved.exact) { + try printSnapNote(color, now_resolved.requested, now_resolved.actual, "now"); + } + now_date = now_resolved.actual; + } + // ── Load both sides ────────────────────────────────────── // // "Then" is always a snapshot. "Now" is either another snapshot // (two-date mode) or the live portfolio (single-date mode). Once // loaded, both sides are shaped identically — a HoldingMap + liquid // total — and feed a single comparison path below. - var then_side = compare_core.loadSnapshotSide(allocator, hist_dir, then_date) catch |err| switch (err) { - error.FileNotFound => { - try suggestNearest(allocator, hist_dir, then_date); - return error.SnapshotNotFound; - }, - else => return err, - }; + // + // After the snap above, the dates are guaranteed to correspond to + // actual snapshot files — FileNotFound here would be a disk race + // (file deleted between the snap check and the load), not a + // missing-snapshot UX problem. + var then_side = try compare_core.loadSnapshotSide(allocator, hist_dir, then_date); defer then_side.deinit(allocator); if (now_is_live) { var now_live = try LiveSide.load(allocator, svc, portfolio_path, color); defer now_live.deinit(allocator); - // Attribution spans then_date → HEAD (or working copy if dirty). - // `computeAttribution` with until=null uses exactly that semantics. - const attribution = contributions_cmd.computeAttribution(allocator, svc, portfolio_path, then_date, null, color); + // Attribution spans then_date_requested → HEAD (or working + // copy if dirty). `computeAttribution` with until=null uses + // exactly that semantics. Using the REQUESTED date (not the + // snapped-to-snapshot date) keeps the git window aligned + // with `zfin contributions --since `. + const attribution = contributions.computeAttribution(allocator, svc, portfolio_path, then_date_requested, null, color); try renderFromParts(out, color, allocator, .{ .then_date = then_date, @@ -176,17 +213,14 @@ pub fn run( .attribution = attribution, }); } else { - var now_side = compare_core.loadSnapshotSide(allocator, hist_dir, now_date) catch |err| switch (err) { - error.FileNotFound => { - try suggestNearest(allocator, hist_dir, now_date); - return error.SnapshotNotFound; - }, - else => return err, - }; + var now_side = try compare_core.loadSnapshotSide(allocator, hist_dir, now_date); defer now_side.deinit(allocator); - // Attribution spans the explicit then_date → now_date window. - const attribution = contributions_cmd.computeAttribution(allocator, svc, portfolio_path, then_date, now_date, color); + // Attribution spans then_date_requested → now_date_requested. + // Using the REQUESTED dates (not the snapped-to-snapshot + // dates) keeps the git window aligned with what `zfin + // contributions --since --until ` would produce. + const attribution = contributions.computeAttribution(allocator, svc, portfolio_path, then_date_requested, now_date_requested, color); try renderFromParts(out, color, allocator, .{ .then_date = then_date, @@ -201,6 +235,32 @@ pub fn run( } } +/// Snap a requested date to the nearest-earlier snapshot that exists +/// on disk. On `NoSnapshotAtOrBefore`, prints a user-facing "no +/// snapshot" error to stderr (with the nearest-later suggestion when +/// available) and propagates the underlying error so the caller can +/// map it to its own domain (e.g. `error.SnapshotNotFound`). +fn printSnapNote(color: bool, requested: Date, actual: Date, label: []const u8) !void { + var req_buf: [10]u8 = undefined; + var act_buf: [10]u8 = undefined; + const req_str = requested.format(&req_buf); + const act_str = actual.format(&act_buf); + const days = requested.days - actual.days; + var msg_buf: [160]u8 = undefined; + const msg = std.fmt.bufPrint( + &msg_buf, + "(requested {s} for {s}; nearest snapshot: {s}, {d} day{s} earlier)\n", + .{ req_str, label, act_str, days, if (days == 1) "" else "s" }, + ) catch "(snapped to nearest snapshot)\n"; + var stderr_buf: [256]u8 = undefined; + var writer = std.fs.File.stderr().writer(&stderr_buf); + const out = &writer.interface; + if (color) try fmt.ansiSetFg(out, cli.CLR_MUTED[0], cli.CLR_MUTED[1], cli.CLR_MUTED[2]); + try out.writeAll(msg); + if (color) try fmt.ansiReset(out); + try out.flush(); +} + /// Inputs needed to build + render a `CompareView`. Bundled into a /// struct so `renderFromParts` stays one line of call-site noise /// instead of an 11-positional-arg parade. @@ -216,7 +276,7 @@ const RenderArgs = struct { now_liquid: f64, then_map: *const view.HoldingMap, now_map: *const view.HoldingMap, - attribution: ?contributions_cmd.AttributionSummary, + attribution: ?contributions.AttributionSummary, }; /// Build the view from two holdings maps + totals, then render. @@ -334,55 +394,6 @@ const LiveSide = struct { } }; -// ── Nearest-snapshot suggestion (stderr, CLI-only) ─────────── - -/// Print a "no snapshot for " message plus the nearest earlier -/// and later available dates to stderr. Wraps the pure -/// `history.findNearestSnapshot` with CLI-specific output. -fn suggestNearest( - allocator: std.mem.Allocator, - hist_dir: []const u8, - target: Date, -) !void { - const nearest = history.findNearestSnapshot(hist_dir, target) catch |err| { - try cli.stderrPrint("Error scanning history directory: "); - try cli.stderrPrint(@errorName(err)); - try cli.stderrPrint("\n"); - return; - }; - - var buf: [128]u8 = undefined; - var target_buf: [10]u8 = undefined; - const target_str = target.format(&target_buf); - - const line = try std.fmt.allocPrint(allocator, "No snapshot for {s}.\n", .{target_str}); - defer allocator.free(line); - try cli.stderrPrint(line); - - if (nearest.earlier == null and nearest.later == null) { - try cli.stderrPrint("No snapshots in "); - try cli.stderrPrint(hist_dir); - try cli.stderrPrint(" — run `zfin snapshot` to create one.\n"); - return; - } - - try cli.stderrPrint("Nearest available:\n"); - if (nearest.earlier) |d| { - var d_buf: [10]u8 = undefined; - const d_str = d.format(&d_buf); - const diff = target.days - d.days; - const msg = try std.fmt.bufPrint(&buf, " earlier: {s} ({d} day{s} before)\n", .{ d_str, diff, view.dayPlural(diff) }); - try cli.stderrPrint(msg); - } - if (nearest.later) |d| { - var d_buf: [10]u8 = undefined; - const d_str = d.format(&d_buf); - const diff = d.days - target.days; - const msg = try std.fmt.bufPrint(&buf, " later: {s} ({d} day{s} after)\n", .{ d_str, diff, view.dayPlural(diff) }); - try cli.stderrPrint(msg); - } -} - // ── ANSI rendering (CLI-only) ──────────────────────────────── // // Thin adapter: pulls pre-formatted cells from `views/compare.zig` @@ -429,6 +440,8 @@ fn renderCompare(out: *std.Io.Writer, color: bool, cv: view.CompareView) !void { for (cv.symbols) |s| { try renderSymbolRow(out, color, s); } + + try renderGainerLoserSummary(out, color, cv); } // Hidden count @@ -442,6 +455,37 @@ fn renderCompare(out: *std.Io.Writer, color: bool, cv: view.CompareView) !void { } } +/// Render the gainer/loser/flat summary line under the per-symbol +/// table. Flat counts only surface when non-zero to keep the signal +/// tight — a full window of winners shouldn't read "0 flat". +/// +/// 21 gainers, 5 losers +/// 21 gainers, 5 losers, 2 flat +/// +/// Colored segments match the per-symbol rows: gainers in the positive +/// intent, losers in the negative intent, "flat" (and punctuation) in +/// the muted intent. "gainer" and "loser" are colored unconditionally +/// — a zero count still communicates something about the window (e.g. +/// "0 losers" in negative tint reinforces "everything was green"). +/// Callers gate on `cv.held_count > 0`. +fn renderGainerLoserSummary(out: *std.Io.Writer, color: bool, cv: view.CompareView) !void { + try cli.printFg(out, color, cli.CLR_MUTED, " ", .{}); + try cli.printFg(out, color, cli.CLR_POSITIVE, "{d} gainer{s}", .{ + cv.gainer_count, + if (cv.gainer_count == 1) "" else "s", + }); + try cli.printFg(out, color, cli.CLR_MUTED, ", ", .{}); + try cli.printFg(out, color, cli.CLR_NEGATIVE, "{d} loser{s}", .{ + cv.loser_count, + if (cv.loser_count == 1) "" else "s", + }); + if (cv.flat_count > 0) { + try cli.printFg(out, color, cli.CLR_MUTED, ", {d} flat\n", .{cv.flat_count}); + } else { + try out.print("\n", .{}); + } +} + fn renderTotalsLine(out: *std.Io.Writer, color: bool, t: view.TotalsRow) !void { var then_buf: [24]u8 = undefined; var now_buf: [24]u8 = undefined; @@ -753,6 +797,145 @@ test "renderCompare: attribution handles negative gains" { try testing.expect(std.mem.indexOf(u8, out, "-$10,000.00") != null); } +test "renderCompare: gainer/loser summary line renders with pluralization" { + const symbols = [_]view.SymbolChange{ + .{ + .symbol = "A", + .price_then = 100, + .price_now = 110, + .shares_held_throughout = 1, + .pct_change = 0.10, + .dollar_change = 10, + .style = .positive, + }, + .{ + .symbol = "B", + .price_then = 100, + .price_now = 105, + .shares_held_throughout = 1, + .pct_change = 0.05, + .dollar_change = 5, + .style = .positive, + }, + .{ + .symbol = "C", + .price_then = 100, + .price_now = 95, + .shares_held_throughout = 1, + .pct_change = -0.05, + .dollar_change = -5, + .style = .negative, + }, + }; + const cv = view.CompareView{ + .then_date = Date.fromYmd(2024, 1, 15), + .now_date = Date.fromYmd(2024, 1, 22), + .days_between = 7, + .now_is_live = true, + .liquid = view.buildTotalsRow(300, 310), + .symbols = @constCast(&symbols), + .held_count = 3, + .added_count = 0, + .removed_count = 0, + .gainer_count = 2, + .loser_count = 1, + .flat_count = 0, + }; + + var buf: [4096]u8 = undefined; + var stream = std.Io.Writer.fixed(&buf); + try renderCompare(&stream, false, cv); + const out = stream.buffered(); + + // Plural gainers, singular loser + try testing.expect(std.mem.indexOf(u8, out, "2 gainers") != null); + try testing.expect(std.mem.indexOf(u8, out, "1 loser") != null); + // Singular "loser" shouldn't have trailing 's' — look for the + // comma-terminated form to disambiguate. + try testing.expect(std.mem.indexOf(u8, out, "1 losers") == null); + // No flat segment when flat_count == 0 + try testing.expect(std.mem.indexOf(u8, out, "flat") == null); +} + +test "renderCompare: gainer/loser summary suppressed when no held symbols" { + const cv = view.CompareView{ + .then_date = Date.fromYmd(2024, 1, 15), + .now_date = Date.fromYmd(2024, 3, 15), + .days_between = 60, + .now_is_live = false, + .liquid = view.buildTotalsRow(100, 110), + .symbols = &.{}, + .held_count = 0, + .added_count = 0, + .removed_count = 0, + }; + + var buf: [2048]u8 = undefined; + var stream = std.Io.Writer.fixed(&buf); + try renderCompare(&stream, false, cv); + const out = stream.buffered(); + + // Neither "gainer" nor "loser" should appear — the summary is + // gated on held_count > 0. + try testing.expect(std.mem.indexOf(u8, out, "gainer") == null); + try testing.expect(std.mem.indexOf(u8, out, "loser") == null); +} + +test "renderCompare: gainer/loser summary includes flat when present" { + const symbols = [_]view.SymbolChange{ + .{ + .symbol = "UP", + .price_then = 100, + .price_now = 110, + .shares_held_throughout = 1, + .pct_change = 0.10, + .dollar_change = 10, + .style = .positive, + }, + .{ + .symbol = "FLAT1", + .price_then = 100, + .price_now = 100, + .shares_held_throughout = 1, + .pct_change = 0, + .dollar_change = 0, + .style = .muted, + }, + .{ + .symbol = "FLAT2", + .price_then = 100, + .price_now = 100, + .shares_held_throughout = 1, + .pct_change = 0, + .dollar_change = 0, + .style = .muted, + }, + }; + const cv = view.CompareView{ + .then_date = Date.fromYmd(2024, 1, 15), + .now_date = Date.fromYmd(2024, 1, 22), + .days_between = 7, + .now_is_live = true, + .liquid = view.buildTotalsRow(300, 310), + .symbols = @constCast(&symbols), + .held_count = 3, + .added_count = 0, + .removed_count = 0, + .gainer_count = 1, + .loser_count = 0, + .flat_count = 2, + }; + + var buf: [4096]u8 = undefined; + var stream = std.Io.Writer.fixed(&buf); + try renderCompare(&stream, false, cv); + const out = stream.buffered(); + + try testing.expect(std.mem.indexOf(u8, out, "1 gainer") != null); + try testing.expect(std.mem.indexOf(u8, out, "0 losers") != null); + try testing.expect(std.mem.indexOf(u8, out, "2 flat") != null); +} + // ── run() entry-point validation tests ───────────────────────── fn makeTestSvc() zfin.DataService { @@ -898,6 +1081,42 @@ test "run: single-date future-date rejected as InvalidDate" { try testing.expectError(error.InvalidDate, result); } +test "run: relative shortcut resolves (1W -> SnapshotNotFound against empty history)" { + // Verifies that `zfin compare 1W` doesn't bail with InvalidDate + // for a non-ISO string — the relative shortcut resolves to an + // absolute date, which then tries to load a snapshot that + // doesn't exist. + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var svc = makeTestSvc(); + defer svc.deinit(); + const pf = try makeTestPortfolioPath(&tmp, testing.allocator); + defer testing.allocator.free(pf); + + var buf: [1024]u8 = undefined; + var stream = std.Io.Writer.fixed(&buf); + + const args = [_][]const u8{"1W"}; + const result = run(testing.allocator, &svc, pf, &args, false, &stream); + try testing.expectError(error.SnapshotNotFound, result); +} + +test "run: 'live' string rejected as InvalidDate (not a real prior date)" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var svc = makeTestSvc(); + defer svc.deinit(); + const pf = try makeTestPortfolioPath(&tmp, testing.allocator); + defer testing.allocator.free(pf); + + var buf: [1024]u8 = undefined; + var stream = std.Io.Writer.fixed(&buf); + + const args = [_][]const u8{"live"}; + const result = run(testing.allocator, &svc, pf, &args, false, &stream); + try testing.expectError(error.InvalidDate, result); +} + test "run: two-date with empty history returns SnapshotNotFound (auto-swap path)" { var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -985,7 +1204,7 @@ fn writeFixtureSnapshot( net_worth: f64, stock_rows: []const snapshot_model.LotRow, ) !void { - const snapshot_cmd = @import("snapshot.zig"); + const snapshot = @import("snapshot.zig"); const totals = [_]snapshot_model.TotalRow{ .{ .kind = "total", .scope = "net_worth", .value = net_worth }, .{ .kind = "total", .scope = "liquid", .value = liquid }, @@ -1005,7 +1224,7 @@ fn writeFixtureSnapshot( .accounts = &.{}, .lots = @constCast(stock_rows), }; - const rendered = try snapshot_cmd.renderSnapshot(allocator, snap); + const rendered = try snapshot.renderSnapshot(allocator, snap); defer allocator.free(rendered); try dir.writeFile(.{ .sub_path = filename, .data = rendered }); } diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index b690f22..9389f4b 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -16,14 +16,70 @@ //! `cli.parseAsOfDate` and resolved to an absolute date before being //! passed in. //! -//! Classifies each lot-level change as: -//! - New contribution (new lot, or cash increase for a fresh cash line) -//! - DRIP / reinvestment (same (symbol, account, open_date, open_price), Δshares) -//! - CD matured (CD removed with maturity_date <= today) -//! - CD removed early (CD removed with maturity_date > today — flagged) -//! - Cash delta (cash shares changed on existing line) -//! - Price-only update (manual price:: field changed, no share change) -//! - Flagged (open_price, maturity, account or other edits) +//! ## Classification matrix +//! +//! Every lot-level change gets exactly one `ChangeKind` assigned at +//! diff time in `computeReport`. Downstream consumers (section printer, +//! per-account summary, `computeAttribution` used by `compare`) all +//! read the pre-classified kinds — there is no post-hoc reclassification +//! in any consumer. Single point of truth so the grand total in +//! `zfin contributions` and the attribution line in `zfin compare` +//! always agree over the same window. +//! +//! ### Base classifications (same for every account) +//! +//! - `new_stock` — stock lot appeared (drip::false) +//! - `new_drip_lot` — stock lot appeared (drip::true → confirmed DRIP) +//! - `new_cash` — cash lot appeared (fresh line, not a balance bump) +//! - `new_cd` — CD opened +//! - `new_option` — option opened +//! - `drip_confirmed` — same-key drip::true stock lot, Δshares > 0 +//! - `rollup_delta` — same-key drip::false stock lot, Δshares > 0 +//! (ambiguous: DRIP or contribution) +//! - `drip_negative` — same-key stock lot, Δshares < 0 (unusual) +//! - `cash_delta` — same-key cash lot, Δshares, default noise +//! - `cd_matured` — CD disappeared, maturity_date ≤ today +//! - `cd_removed_early` — CD disappeared, maturity_date > today +//! - `lot_removed` — stock/cash/option lot disappeared +//! - `lot_edited` — secondary-key match across broken strict keys +//! (open_date/open_price/symbol-alias rewrite) +//! - `price_only` — same-key, only the `price::` field changed +//! - `flagged` — any other edit shape (maturity_date change, etc.) +//! +//! ### Cash-account opt-in (`cash_is_contribution::true` in accounts.srf) +//! +//! Most cash-account activity is internal flow — DRIP cash legs, +//! dividend credits, CD coupons, settlement sweeps — which is why +//! `cash_delta` is noise by default. But for payroll-adjacent +//! accounts (ESPP accrual, direct 401k cash deposits, HSA employer +//! contributions), a positive cash movement IS the contribution. +//! The `cash_is_contribution::true` flag in `accounts.srf` opts an +//! account into "positive cash_delta = real contribution" semantics. +//! +//! Opted-IN accounts: +//! +//! | Scenario | Kind | Section | In Grand Total | In Attribution | +//! |-------------------------------|--------------------|----------------------|:--------------:|:--------------:| +//! | Brand-new cash lot appears | `new_cash` | New contributions | yes | yes | +//! | Existing cash, balance up | `cash_contribution`| New contributions | yes | yes | +//! | Existing cash, balance down | `cash_delta` | Cash deltas (raw) | no | no | +//! | Cash lot fully removed | `lot_removed` | Flagged for review | no | no | +//! +//! Opted-OUT accounts (default): +//! +//! | Scenario | Kind | Section | In Grand Total | In Attribution | +//! |-------------------------------|--------------------|----------------------|:--------------:|:--------------:| +//! | Brand-new cash lot appears | `new_cash` | New contributions | yes | yes | +//! | Existing cash, balance moves | `cash_delta` | Cash deltas (raw) | no | no | +//! | Cash lot fully removed | `lot_removed` | Flagged for review | no | no | +//! +//! The asymmetry on negative Δ for opted-in accounts is intentional: +//! a withdrawal from ESPP/HSA is rare and semantically different +//! from "a contribution of negative money." The branch for that +//! (withdraw-as-negative-contribution) is one `if (delta < 0)` in +//! `computeReport` if/when the need arises. +//! +//! ## Other architecture notes //! //! Relies on: portfolio.srf being tracked in a git repo, and the `git` //! executable existing on PATH. We never rely on comments; maturity is @@ -33,6 +89,7 @@ const std = @import("std"); const zfin = @import("../root.zig"); const cli = @import("common.zig"); const git = @import("../git.zig"); +const analysis = @import("../analytics/analysis.zig"); const fmt = cli.fmt; const Date = zfin.Date; const Lot = zfin.Lot; @@ -211,7 +268,21 @@ fn prepareReport( } } - const report = computeReport(arena, before_pf.lots, after_pf.lots, &prices, fmt.todayDate()) catch { + // Load accounts.srf (if present) so opt-in cash-delta + // reclassification fires at diff time. When missing or + // unparseable, computeReport falls back to the default cash_delta + // classification — no account gets the opt-in treatment. + var account_map_opt = svc.loadAccountMap(portfolio_path); + defer if (account_map_opt) |*am| am.deinit(); + + const report = computeReport( + arena, + before_pf.lots, + after_pf.lots, + &prices, + fmt.todayDate(), + .{ .account_map = if (account_map_opt) |*am| am else null }, + ) catch { if (verbosity == .verbose) cli.stderrPrint("Error computing contributions diff.\n") catch {}; return error.PrepareFailed; }; @@ -374,6 +445,14 @@ pub const AttributionSummary = struct { /// Parameters mirror `run` but without the writer/color: no output /// is produced. Failures swallow silently via the shared /// `prepareReport` helper's `.silent` verbosity. +/// +/// Classification logic is SOLELY in `computeReport` — this function +/// just buckets pre-classified change kinds into their totals. In +/// particular, opt-in cash-delta handling is resolved at diff time +/// (cash_delta → cash_contribution on accounts marked +/// `cash_is_contribution::true`), so the attribution line here and +/// the grand total in the full `zfin contributions` report come out +/// of the same classifier and always agree over the same window. pub fn computeAttribution( allocator: std.mem.Allocator, svc: *zfin.DataService, @@ -397,15 +476,20 @@ pub fn computeAttribution( // Aggregate. Classification logic matches the full-report sections: // - New contributions: new_stock + new_cash + new_cd + new_option + // + cash_contribution (opt-in cash_delta) // - DRIP: new_drip_lot + drip_confirmed + rollup_delta // `rollup_delta` is the ambiguous "share increased on a drip::false // lot" case. Lumping it with DRIP here matches the report's own // visual grouping (both shown as positive, both under DRIP-ish // headings) and prevents double-counting against cash_delta. + // `cash_delta` (non-opt-in) is excluded — it's noisy cash movement + // (DRIP legs, interest, CD coupons, settlement sweeps) that the + // user hasn't explicitly flagged as a contribution source. + // `lot_edited` is excluded — it's a reconciliation noise bucket. var new_contributions: f64 = 0; var drip: f64 = 0; for (ctx.report.changes) |c| switch (c.kind) { - .new_stock, .new_cash, .new_cd, .new_option => new_contributions += c.value(), + .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => new_contributions += c.value(), .new_drip_lot, .drip_confirmed, .rollup_delta => drip += c.value(), else => {}, }; @@ -431,10 +515,27 @@ const ChangeKind = enum { drip_confirmed, // same key on a drip::true stock lot, Δshares > 0 rollup_delta, // same key on a drip::false stock lot, Δshares > 0 (DRIP or contribution; can't distinguish) drip_negative, // same key, stock, Δshares < 0 (share sale on the same lot — unusual) - cash_delta, // same key, cash, Δshares + cash_delta, // same key, cash, Δshares (treated as noise — interest, DRIP legs) + /// Positive cash_delta on an account marked + /// `cash_is_contribution::true` in accounts.srf. Reclassified at + /// diff time (inside `computeReport`) so every downstream + /// consumer — the full report, per-account summary, attribution + /// totals in `compare` — sees the same classification. Without + /// this single-point-of-truth, compare's attribution line and the + /// contributions report's grand total could disagree on the same + /// window. + cash_contribution, cd_matured, // lot disappeared: CD with maturity_date <= today cd_removed_early, // lot disappeared: CD with maturity_date > today lot_removed, // lot disappeared: stock/cash/option + /// Strict lot key broke (open_date / open_price / account rewritten) + /// but the same (security_type, symbol, account) reappears on the + /// other side with approximately the same share total. Classified + /// as a lot edit — not counted as contribution, not counted as + /// disposal. Fixes the phantom-contribution bug from reconciliation + /// tweaks, CD auto-renewals rewriting `open_date`, and account + /// renames that shift every lot in the account under a new name. + lot_edited, price_only, // same key, price:: field changed, no share change flagged, // any other shape of edit }; @@ -517,18 +618,242 @@ fn aggregateByKey( return map; } +/// Secondary key for edit detection: (security_type, priceSymbol, account). +/// Lots with the same secondary key but different strict `lotKey`s are +/// candidates for reclassification as `lot_edited` — the strict key +/// broke because `open_date`, `open_price`, or the underlying symbol +/// string got rewritten, but the position itself continued. +/// +/// Uses `priceSymbol()` (ticker-alias-aware) rather than raw `symbol` +/// so that edits like `symbol::SPY` → `symbol::DI-SPX, ticker::SPY` +/// collapse correctly. Both sides resolve to the same effective +/// ticker (`SPY`) and represent the same underlying exposure; the +/// raw `symbol` changed but the position did not. +fn secondaryKey(allocator: std.mem.Allocator, lot: Lot) ![]u8 { + return std.fmt.allocPrint(allocator, "{s}|{s}|{s}", .{ + @tagName(lot.security_type), + lot.priceSymbol(), + lot.account orelse "", + }); +} + +/// Tolerance for "did the share total stay the same" check when +/// deciding whether to emit a rollup_delta for any residual share +/// difference alongside a `lot_edited` classification. Anything within +/// this tolerance is treated as rounding/reconciliation noise and +/// suppressed; anything beyond it surfaces as a normal share-delta +/// change (rollup_delta for positive, drip_negative for negative). +/// +/// This is NOT a gate on whether the pair collapses — secondary-key +/// matches ALWAYS collapse the identity change into a lot_edited +/// record. The tolerance only decides whether to additionally emit +/// the residual share movement as a distinct change. +const edit_residual_tolerance_rel: f64 = 0.0001; // 0.01% + +/// Identify strict-key pairs that should be reclassified as edits. +/// +/// Walks the "only in after" and "only in before" strict keys, groups +/// each by secondary key (security_type, priceSymbol, account), and +/// returns a set of strict keys that should be skipped by the primary +/// classification passes (both `new_*` and `lot_removed` / `cd_*`). +/// Populates `changes` with a single `lot_edited` entry per matched +/// secondary-key group, plus a residual `rollup_delta` / `drip_negative` +/// for any share difference beyond noise tolerance. +/// +/// Matching rules: +/// - A secondary key must have at least one unmatched lot on each +/// side (otherwise it's a pure add or a pure remove, which keeps +/// the existing semantics). +/// - All unmatched strict keys for that secondary key group collapse +/// together regardless of share-total magnitude. The thinking: +/// secondary-key agreement (same `(security_type, priceSymbol, +/// account)`) means the user is editing the same underlying +/// position. Any share difference is a separate question — +/// handled by emitting a residual rollup_delta (if positive) or +/// drip_negative (if negative). This matches how share deltas on +/// intact strict keys are classified in Pass 1. +fn detectEdits( + allocator: std.mem.Allocator, + before_map: *const std.StringHashMap(LotAgg), + after_map: *const std.StringHashMap(LotAgg), + prices: *const std.StringHashMap(f64), + changes: *std.ArrayList(Change), +) !std.StringHashMap(void) { + var skip = std.StringHashMap(void).init(allocator); + errdefer skip.deinit(); + + // Group unmatched strict keys by secondary key, tagging each entry + // with the side it came from. `strict_keys` alongside the value + // lets us seed the skip set only when a group actually matches. + const SidedEntry = struct { + strict_key: []const u8, + agg: LotAgg, + from_after: bool, + }; + var groups = std.StringHashMap(std.ArrayList(SidedEntry)).init(allocator); + defer { + var vit = groups.valueIterator(); + while (vit.next()) |list| list.deinit(allocator); + groups.deinit(); + } + + var ait = after_map.iterator(); + while (ait.next()) |entry| { + if (before_map.contains(entry.key_ptr.*)) continue; + const sk = try secondaryKey(allocator, entry.value_ptr.*.lot); + const gop = try groups.getOrPut(sk); + if (!gop.found_existing) { + gop.value_ptr.* = std.ArrayList(SidedEntry).empty; + } else { + allocator.free(sk); + } + try gop.value_ptr.append(allocator, .{ + .strict_key = entry.key_ptr.*, + .agg = entry.value_ptr.*, + .from_after = true, + }); + } + + var bit = before_map.iterator(); + while (bit.next()) |entry| { + if (after_map.contains(entry.key_ptr.*)) continue; + const sk = try secondaryKey(allocator, entry.value_ptr.*.lot); + const gop = try groups.getOrPut(sk); + if (!gop.found_existing) { + gop.value_ptr.* = std.ArrayList(SidedEntry).empty; + } else { + allocator.free(sk); + } + try gop.value_ptr.append(allocator, .{ + .strict_key = entry.key_ptr.*, + .agg = entry.value_ptr.*, + .from_after = false, + }); + } + + // Dup helper for string arena storage into `changes`. + const Dup = struct { + a: std.mem.Allocator, + fn of(self: @This(), s: []const u8) ![]const u8 { + return self.a.dupe(u8, s); + } + }; + const sdup = Dup{ .a = allocator }; + + var git_it = groups.iterator(); + while (git_it.next()) |entry| { + const list = entry.value_ptr.*; + + // Need at least one on each side to be an edit. + var after_shares: f64 = 0; + var before_shares: f64 = 0; + var after_rep: ?LotAgg = null; + var before_rep: ?LotAgg = null; + for (list.items) |e| { + if (e.from_after) { + after_shares += e.agg.shares; + if (after_rep == null) after_rep = e.agg; + } else { + before_shares += e.agg.shares; + if (before_rep == null) before_rep = e.agg; + } + } + if (after_shares == 0 or before_shares == 0) continue; + + // Qualified edit: add all strict keys to the skip set. Emit a + // lot_edited record representing the identity continuity. + for (list.items) |e| { + try skip.put(e.strict_key, {}); + } + + const rep_lot = after_rep.?.lot; + const acct = try sdup.of(rep_lot.account orelse ""); + const sym = try sdup.of(rep_lot.symbol); + try changes.append(allocator, .{ + .kind = .lot_edited, + .symbol = sym, + .account = acct, + .security_type = rep_lot.security_type, + .delta_shares = 0, + .unit_value = 0, + }); + + // Emit a residual share-delta change if the totals diverge + // beyond noise. Mirrors Pass 1's same-key share-delta handling + // so the user sees the same classification whether the strict + // key was preserved or rewritten. + const delta = after_shares - before_shares; + const denom = @max(@abs(after_shares), @abs(before_shares)); + const rel = if (denom == 0) 0.0 else @abs(delta) / denom; + if (rel <= edit_residual_tolerance_rel) continue; + + const unit_value: f64 = blk: { + if (rep_lot.security_type == .stock) { + if (prices.get(rep_lot.priceSymbol())) |p| break :blk p * rep_lot.price_ratio; + if (rep_lot.price) |p| break :blk p * rep_lot.price_ratio; + break :blk rep_lot.open_price * rep_lot.price_ratio; + } + break :blk 1.0; + }; + + const is_drip = rep_lot.drip or (before_rep.?.lot.drip); + const kind: ChangeKind = switch (rep_lot.security_type) { + .stock => if (delta > 0) + (if (is_drip) ChangeKind.drip_confirmed else ChangeKind.rollup_delta) + else + ChangeKind.drip_negative, + .cash => .cash_delta, + else => .flagged, + }; + try changes.append(allocator, .{ + .kind = kind, + .symbol = sym, + .account = acct, + .security_type = rep_lot.security_type, + .delta_shares = delta, + .unit_value = unit_value, + }); + } + + return skip; +} + +/// Optional knobs for `computeReport`. An explicit struct keeps the +/// call-site noise low at the many in-file test call sites (all pass +/// `.{}`) while still letting production callers thread through the +/// account map for opted-in cash-delta reclassification. +const ReportOptions = struct { + /// Account metadata from `accounts.srf`. When present, positive + /// `cash_delta` entries for accounts with + /// `cash_is_contribution::true` are reclassified as + /// `cash_contribution` so every downstream consumer (full + /// report, per-account summary, `compare` attribution) agrees. + account_map: ?*const analysis.AccountMap = null, +}; + fn computeReport( allocator: std.mem.Allocator, before: []const Lot, after: []const Lot, prices: *const std.StringHashMap(f64), today: Date, + opts: ReportOptions, ) !Report { var changes: std.ArrayList(Change) = .empty; var before_map = try aggregateByKey(allocator, before); var after_map = try aggregateByKey(allocator, after); + // Edit detection: identify strict-key pairs that look like edits + // (reconciliation tweak, CD auto-renewal rewriting `open_date`, + // symbol alias rewrite like SPY→DI-SPX,ticker::SPY) rather than + // real new/removed lots. The returned `skip` set is the list of + // strict keys passes 1 and 2 should ignore — each matched group + // emits its own `lot_edited` change plus a residual rollup for + // any share delta beyond noise. + var skip = try detectEdits(allocator, &before_map, &after_map, prices, &changes); + defer skip.deinit(); + // Helper for duping strings into the arena so Change fields have // predictable lifetimes even if caller-supplied lot strings go away. const Dup = struct { @@ -543,6 +868,7 @@ fn computeReport( // with price-only or other-metadata changes) edited. var ait = after_map.iterator(); while (ait.next()) |entry| { + if (skip.contains(entry.key_ptr.*)) continue; const after_agg = entry.value_ptr.*; if (before_map.get(entry.key_ptr.*)) |before_agg| { // Key present in both. Compare shares and other fields. @@ -553,7 +879,7 @@ fn computeReport( if (@abs(delta) > 0.000001) { const before_lot = before_agg.lot; const is_drip = lot.drip or before_lot.drip; - const kind: ChangeKind = switch (lot.security_type) { + const base_kind: ChangeKind = switch (lot.security_type) { .stock => if (delta > 0) (if (is_drip) ChangeKind.drip_confirmed else ChangeKind.rollup_delta) else @@ -563,6 +889,21 @@ fn computeReport( .option => .flagged, else => .flagged, }; + // Opt-in reclassification: on accounts marked + // `cash_is_contribution::true` in accounts.srf, a + // positive cash_delta is actually new money arriving + // (payroll ESPP accrual, direct 401k cash deposit, + // etc.). Reclassify at this single point so every + // downstream consumer — full report, per-account + // summary, `compare` attribution — sees the same + // classification. Negative cash_delta stays as noise + // (a real withdrawal would need different semantics). + const kind: ChangeKind = if (base_kind == .cash_delta and delta > 0) blk: { + if (opts.account_map) |am| { + if (am.cashIsContribution(lot.account orelse "")) break :blk .cash_contribution; + } + break :blk .cash_delta; + } else base_kind; // Determine unit_value for stocks: prefer current cached price; // fall back to manual price::; fall back to open_price. const unit_value: f64 = blk: { @@ -655,6 +996,7 @@ fn computeReport( var bit = before_map.iterator(); while (bit.next()) |entry| { if (after_map.contains(entry.key_ptr.*)) continue; + if (skip.contains(entry.key_ptr.*)) continue; const before_agg = entry.value_ptr.*; const lot = before_agg.lot; const acct = try sdup.of(lot.account orelse ""); @@ -692,7 +1034,7 @@ fn computeReport( const gop = try acct_totals.getOrPut(c.account); if (!gop.found_existing) gop.value_ptr.* = .{}; switch (c.kind) { - .new_stock, .new_cash, .new_cd, .new_option => { + .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => { gop.value_ptr.new_money += c.value(); }, .new_drip_lot, .drip_confirmed => { @@ -746,7 +1088,7 @@ fn printReport(out: *std.Io.Writer, report: *const Report, label: []const u8, co var any = false; var new_total: f64 = 0; for (report.changes) |c| switch (c.kind) { - .new_stock, .new_cash, .new_cd, .new_option => { + .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => { any = true; new_total += c.value(); try printChangeLine(out, c, color, pos_color); @@ -849,6 +1191,36 @@ fn printReport(out: *std.Io.Writer, report: *const Report, label: []const u8, co try out.writeAll("\n"); } + // ── Section: Lot edits (reclassified, not counted) ── + // + // Broken strict lot keys that matched a secondary key with + // approximately-equal share totals. Shown as a muted section so + // the user can verify the reclassification was correct (e.g. a CD + // auto-renewed `open_date`, an account got renamed, or the + // tax-loss account had shares tweaked during reconciliation). + // Not counted as contributions, DRIP, or removals. + var any_edit = false; + for (report.changes) |c| if (c.kind == .lot_edited) { + any_edit = true; + break; + }; + if (any_edit) { + try printSection(out, "Lot edits (same position, key rewritten — not counted)", color, h_color); + for (report.changes) |c| switch (c.kind) { + .lot_edited => { + var buf: [256]u8 = undefined; + const msg = std.fmt.bufPrint( + &buf, + " {s: <12} {s: <24} (strict key broke, shares unchanged)\n", + .{ c.symbol, c.account }, + ) catch " (lot edit)\n"; + try cli.printFg(out, color, mut_color, "{s}", .{msg}); + }, + else => {}, + }; + try out.writeAll("\n"); + } + // ── Section: Flagged ── var any_flag = false; for (report.changes) |c| switch (c.kind) { @@ -910,12 +1282,18 @@ fn printReport(out: *std.Io.Writer, report: *const Report, label: []const u8, co var buf2: [32]u8 = undefined; var buf3: [32]u8 = undefined; var buf4: [32]u8 = undefined; + var buf_grand: [32]u8 = undefined; try out.print(" New contributions / purchases: {s}\n", .{fmt.fmtMoneyAbs(&buf1, total_new)}); try out.print(" DRIP (confirmed): {s}\n", .{fmt.fmtMoneyAbs(&buf2, total_drip)}); try out.print(" Rollup share deltas: {s} (DRIP or contribution; can't distinguish)\n", .{fmt.fmtMoneyAbs(&buf3, total_rollup)}); if (total_cd_int > 0) { try out.print(" CD interest captured: {s}\n", .{fmt.fmtMoneyAbs(&buf4, total_cd_int)}); } + // Grand total across everything "money in"-ish. CD interest is + // included because it's real return realized during the window, + // even though it originated inside the portfolio. + const grand = total_new + total_drip + total_rollup + total_cd_int; + try cli.printFg(out, color, h_color, " Grand total: {s}\n", .{fmt.fmtMoneyAbs(&buf_grand, grand)}); } fn printSection(out: *std.Io.Writer, title: []const u8, color: bool, hdr: [3]u8) !void { @@ -1055,7 +1433,7 @@ test "computeReport: fresh stock purchase counts as new contribution" { .{ .symbol = "AAPL", .shares = 10, .open_date = Date.fromYmd(2026, 4, 1), .open_price = 180, .account = "Brokerage" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.new_stock, report.changes[0].kind); @@ -1077,7 +1455,7 @@ test "computeReport: rollup_delta when shares increase on untagged lot" { .{ .symbol = "VBTLX", .shares = 1010, .open_date = Date.fromYmd(2026, 2, 26), .open_price = 9.79, .account = "DCP" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); // drip::false on both sides → rollup_delta (ambiguous: DRIP or contribution) @@ -1098,7 +1476,7 @@ test "computeReport: matured CD with maturity_date <= today" { }; const after = [_]Lot{}; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.cd_matured, report.changes[0].kind); @@ -1117,7 +1495,7 @@ test "computeReport: CD removed before maturity flagged as early" { }; const after = [_]Lot{}; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.cd_removed_early, report.changes[0].kind); @@ -1137,7 +1515,7 @@ test "computeReport: price-only update not classified as cash flow" { .{ .symbol = "NON40OR52", .shares = 5000, .open_date = Date.fromYmd(2026, 2, 26), .open_price = 97.24, .price = 169.07, .price_date = Date.fromYmd(2026, 4, 18), .account = "401k" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.price_only, report.changes[0].kind); @@ -1161,7 +1539,7 @@ test "computeReport: CD matured + cash increase -> implied interest available" { .{ .symbol = "CASH", .shares = 62510.95, .open_date = Date.fromYmd(2026, 2, 25), .open_price = 1.0, .security_type = .cash, .account = "IRA" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); // Expect: 1 cd_matured, 1 cash_delta var n_matured: usize = 0; @@ -1202,7 +1580,7 @@ test "computeReport: unit_value prefers current price over open_price for rollup .{ .symbol = "SPY", .shares = 718.4848, .open_date = Date.fromYmd(2026, 2, 25), .open_price = 461.24, .account = "Tax Loss" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.rollup_delta, report.changes[0].kind); @@ -1225,7 +1603,7 @@ test "computeReport: manual-priced lot (price:: no ticker) uses manual price" { .{ .symbol = "NON40OR52", .shares = 5075.077, .open_date = Date.fromYmd(2026, 2, 26), .open_price = 97.24, .price = 169.07, .account = "401k" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.rollup_delta, report.changes[0].kind); @@ -1247,12 +1625,399 @@ test "computeReport: maturity_date change on same CD is flagged" { .{ .symbol = "CDY", .shares = 87000, .open_date = Date.fromYmd(2026, 2, 25), .open_price = 1.0, .security_type = .cd, .account = "IRA", .maturity_date = Date.fromYmd(2027, 7, 15) }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.flagged, report.changes[0].kind); } +test "computeReport: CD open_date rewrite reclassified as edit, not new+removed" { + // The classic CD auto-renewal scenario: shares and account stay + // the same, but the `open_date` gets rewritten to the renewal + // date. Prior behavior was a phantom $58k new_cd contribution + // plus a $58k cd_removed_early. After edit detection: one + // lot_edited, no contribution. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const before = [_]Lot{ + .{ .symbol = "CD1", .shares = 58000, .open_date = Date.fromYmd(2026, 2, 25), .open_price = 1.0, .security_type = .cd, .account = "IRA", .maturity_date = Date.fromYmd(2027, 2, 25) }, + }; + const after = [_]Lot{ + // Same CD, rewritten open_date (e.g. renewal) — key broken. + .{ .symbol = "CD1", .shares = 58000, .open_date = Date.fromYmd(2026, 4, 20), .open_price = 1.0, .security_type = .cd, .account = "IRA", .maturity_date = Date.fromYmd(2027, 4, 20) }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.lot_edited, report.changes[0].kind); + + // Attribution must see zero contribution for this CD. + var new_contrib: f64 = 0; + var drip: f64 = 0; + for (report.changes) |c| switch (c.kind) { + .new_stock, .new_cash, .new_cd, .new_option => new_contrib += c.value(), + .new_drip_lot, .drip_confirmed, .rollup_delta => drip += c.value(), + else => {}, + }; + try std.testing.expectApproxEqAbs(@as(f64, 0), new_contrib, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0), drip, 0.01); +} + +test "computeReport: stock open_price renormalized reclassified as edit" { + // Reconciliation tweak: user updates `open_price` to match the + // institutional-share-class NAV, leaving everything else alone. + // Prior behavior: phantom new_stock for the full lot value. + // After edit detection: lot_edited. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("TAXLOSS", 50.0); + + const before = [_]Lot{ + .{ .symbol = "TAXLOSS", .shares = 100, .open_date = Date.fromYmd(2026, 1, 15), .open_price = 45.0, .account = "Tax Loss" }, + }; + const after = [_]Lot{ + .{ .symbol = "TAXLOSS", .shares = 100, .open_date = Date.fromYmd(2026, 1, 15), .open_price = 48.0, .account = "Tax Loss" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.lot_edited, report.changes[0].kind); +} + +test "computeReport: symbol rename with ticker alias collapses to lot_edited" { + // 2026-05-02 regression: user changed a tax-loss lot from + // `symbol::SPY` to `symbol::DI-SPX, ticker::SPY` (direct-indexing + // proxy) and tweaked shares by ~1% during reconciliation. Same + // underlying SPY exposure, same account, same open_date / + // open_price — should collapse to an edit, NOT a ~$327k phantom + // contribution. + // + // Before the `priceSymbol()`-based secondary key, the raw-symbol + // comparison treated "SPY" and "DI-SPX" as different positions + // and emitted a `new_stock` + `lot_removed` pair worth the full + // lot value (~715 × $461 = $330k). + // + // After the fix: one `lot_edited` for the identity continuity, + // plus a `rollup_delta` for the ~6-share residual worth ~$3k. + // The attribution total sees only the residual, not the full lot. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SPY", 461.24); + + const before = [_]Lot{ + .{ + .symbol = "SPY", + .shares = 715.912037, + .open_date = Date.fromYmd(2026, 2, 25), + .open_price = 461.240208, + .account = "Tax Loss", + }, + }; + const after = [_]Lot{ + .{ + .symbol = "DI-SPX", + .ticker = "SPY", + .shares = 709.235272, // ~1% tweak during reconciliation + .open_date = Date.fromYmd(2026, 2, 25), + .open_price = 461.240208, + .account = "Tax Loss", + .price_ratio = 1.0, + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + // Expect one lot_edited + one residual share-delta change + // (rollup_delta is used here because delta < 0 would be + // drip_negative — but 709 < 715 means after < before, so + // drip_negative). Actually delta = 709 - 715 = -6, so + // drip_negative. + var n_edit: usize = 0; + var n_rollup: usize = 0; + var n_drip_neg: usize = 0; + var n_new_stock: usize = 0; + var n_removed: usize = 0; + var residual_value: f64 = 0; + for (report.changes) |c| switch (c.kind) { + .lot_edited => n_edit += 1, + .rollup_delta => { + n_rollup += 1; + residual_value = c.value(); + }, + .drip_negative => { + n_drip_neg += 1; + residual_value = c.value(); + }, + .new_stock => n_new_stock += 1, + .lot_removed => n_removed += 1, + else => {}, + }; + try std.testing.expectEqual(@as(usize, 1), n_edit); + try std.testing.expectEqual(@as(usize, 0), n_new_stock); + try std.testing.expectEqual(@as(usize, 0), n_removed); + // Share count went DOWN, so we get a drip_negative for the + // residual (not rollup_delta). + try std.testing.expectEqual(@as(usize, 1), n_drip_neg); + try std.testing.expectEqual(@as(usize, 0), n_rollup); + // ~6.68 shares × 461.24 ≈ $3,080 — the real reconciliation-scale + // movement, not the ~$330k phantom. + try std.testing.expect(residual_value < 0); // drip_negative sign + try std.testing.expect(@abs(residual_value) < 5_000); // nowhere near $330k +} + +test "computeReport: ticker-alias removed (CUSIP-like -> plain ticker) also collapses" { + // Reverse direction: before has a CUSIP-style symbol with ticker + // alias, after has the plain ticker with no alias. Both resolve + // to the same `priceSymbol()` → edit, not new+removed. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("VTTHX", 27.78); + + const before = [_]Lot{ + .{ + .symbol = "00766V100", // fake CUSIP-style string + .ticker = "VTTHX", + .shares = 5000, + .open_date = Date.fromYmd(2023, 1, 15), + .open_price = 25.0, + .account = "401k", + }, + }; + const after = [_]Lot{ + .{ + .symbol = "VTTHX", + .shares = 5000, + .open_date = Date.fromYmd(2023, 1, 15), + .open_price = 25.0, + .account = "401k", + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.lot_edited, report.changes[0].kind); +} + +test "computeReport: different tickers stay distinct (no false collapse)" { + // Sanity: VOO → VTI in the same account with a broken strict key + // is NOT the same underlying position. Must not collapse into an + // edit. Should classify as new + removed. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("VOO", 450.0); + try prices.put("VTI", 230.0); + + const before = [_]Lot{ + .{ + .symbol = "VOO", + .shares = 100, + .open_date = Date.fromYmd(2024, 1, 15), + .open_price = 400.0, + .account = "IRA", + }, + }; + const after = [_]Lot{ + .{ + .symbol = "VTI", + .shares = 100, + .open_date = Date.fromYmd(2024, 1, 15), + .open_price = 200.0, + .account = "IRA", + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + var n_edit: usize = 0; + var n_new: usize = 0; + var n_removed: usize = 0; + for (report.changes) |c| switch (c.kind) { + .lot_edited => n_edit += 1, + .new_stock => n_new += 1, + .lot_removed => n_removed += 1, + else => {}, + }; + try std.testing.expectEqual(@as(usize, 0), n_edit); + try std.testing.expectEqual(@as(usize, 1), n_new); + try std.testing.expectEqual(@as(usize, 1), n_removed); +} + +test "computeReport: account rename is NOT collapsed (documented limitation)" { + // Renaming an account string in portfolio.srf (e.g. "Brokerage" → + // "Joint Brokerage") breaks the secondary key too, since that key + // includes the account. Edit detection DOES NOT cover this case + // — the rename looks indistinguishable from a transfer (closing + // one account, opening another with the same positions). Account + // renames must therefore be handled manually: either avoid them + // during a review window, or accept the phantom attribution for + // that week. Tracked in TODO.md. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("AAPL", 180.0); + try prices.put("MSFT", 400.0); + + const before = [_]Lot{ + .{ .symbol = "AAPL", .shares = 100, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 150.0, .account = "Brokerage" }, + .{ .symbol = "MSFT", .shares = 50, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 300.0, .account = "Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "AAPL", .shares = 100, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 150.0, .account = "Joint Brokerage" }, + .{ .symbol = "MSFT", .shares = 50, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 300.0, .account = "Joint Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + // Expectation: account rename collapses into regular new+removed + // classification (NOT lot_edited). If this ever flips to 2/0/0, + // edit detection has gained account-rename awareness — great, + // but update this test and the TODO accordingly. + var n_edit: usize = 0; + var n_new: usize = 0; + var n_removed: usize = 0; + for (report.changes) |c| switch (c.kind) { + .lot_edited => n_edit += 1, + .new_stock => n_new += 1, + .lot_removed => n_removed += 1, + else => {}, + }; + try std.testing.expectEqual(@as(usize, 0), n_edit); + try std.testing.expectEqual(@as(usize, 2), n_new); + try std.testing.expectEqual(@as(usize, 2), n_removed); +} + +test "computeReport: big share delta with broken key emits lot_edited + residual rollup" { + // Secondary key matches (same security_type + priceSymbol + + // account) but the share total diverges significantly. This is + // the "user broke the lot key AND had a real contribution in + // the same window" case. The lot identity still collapses to an + // edit, but the share delta surfaces as a rollup_delta so the + // attribution total sees the real inflow — not the full lot + // value as a phantom contribution. + // + // Prior behavior (1% share tolerance): fell through to + // new_stock + lot_removed, over-counting by the full lot value. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("AAPL", 180.0); + + const before = [_]Lot{ + .{ .symbol = "AAPL", .shares = 100, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 150.0, .account = "Brokerage" }, + }; + const after = [_]Lot{ + // Same symbol/account but BIG share delta and different + // open_date. The 400-share delta is real new money; the + // 100-share continuation is an edit. + .{ .symbol = "AAPL", .shares = 500, .open_date = Date.fromYmd(2026, 4, 15), .open_price = 175.0, .account = "Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + var n_edit: usize = 0; + var n_rollup: usize = 0; + var n_new: usize = 0; + var n_removed: usize = 0; + var rollup_value: f64 = 0; + for (report.changes) |c| switch (c.kind) { + .lot_edited => n_edit += 1, + .rollup_delta => { + n_rollup += 1; + rollup_value += c.value(); + }, + .new_stock => n_new += 1, + .lot_removed => n_removed += 1, + else => {}, + }; + try std.testing.expectEqual(@as(usize, 1), n_edit); + try std.testing.expectEqual(@as(usize, 1), n_rollup); + try std.testing.expectEqual(@as(usize, 0), n_new); + try std.testing.expectEqual(@as(usize, 0), n_removed); + // 400-share delta × $180 current price = $72k + try std.testing.expectApproxEqAbs(@as(f64, 72_000.0), rollup_value, 1.0); +} + +test "computeReport: tiny share drift emits lot_edited + residual rollup" { + // Fractional DRIP share-count, e.g. 10.0 → 10.05 with a + // reconciliation tweak and a key rewrite. Under the always-collapse + // design, this produces lot_edited + a small rollup_delta for + // the 0.05-share residual. The residual survives the 0.01% noise + // tolerance because 0.5% > 0.01%. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SPY", 500.0); + + const before = [_]Lot{ + .{ .symbol = "SPY", .shares = 10.0, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 420.0, .account = "IRA" }, + }; + const after = [_]Lot{ + .{ .symbol = "SPY", .shares = 10.05, .open_date = Date.fromYmd(2023, 8, 1), .open_price = 430.0, .account = "IRA" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + var n_edit: usize = 0; + var n_rollup: usize = 0; + for (report.changes) |c| switch (c.kind) { + .lot_edited => n_edit += 1, + .rollup_delta => n_rollup += 1, + else => {}, + }; + try std.testing.expectEqual(@as(usize, 1), n_edit); + try std.testing.expectEqual(@as(usize, 1), n_rollup); +} + +test "computeReport: sub-noise share drift emits only lot_edited" { + // If the share delta is below the residual-tolerance threshold + // (0.01%), we emit only lot_edited — no spurious rollup_delta + // from floating-point noise. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SPY", 500.0); + + const before = [_]Lot{ + .{ .symbol = "SPY", .shares = 1000.0, .open_date = Date.fromYmd(2023, 6, 1), .open_price = 420.0, .account = "IRA" }, + }; + const after = [_]Lot{ + // 0.00005% drift (well under 0.01% tolerance) + .{ .symbol = "SPY", .shares = 1000.0000005, .open_date = Date.fromYmd(2023, 8, 1), .open_price = 430.0, .account = "IRA" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 21), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.lot_edited, report.changes[0].kind); +} + test "computeReport: new lot with drip::true classified as new_drip_lot" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); @@ -1265,7 +2030,7 @@ test "computeReport: new lot with drip::true classified as new_drip_lot" { .{ .symbol = "FAGIX", .shares = 10.0, .open_date = Date.fromYmd(2026, 4, 10), .open_price = 11.00, .account = "Kelly IRA", .drip = true }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.new_drip_lot, report.changes[0].kind); @@ -1283,8 +2048,7 @@ test "computeReport: new stock lot without drip flag is new_stock" { const after = [_]Lot{ .{ .symbol = "AAPL", .shares = 5, .open_date = Date.fromYmd(2026, 4, 10), .open_price = 180, .account = "Brokerage" }, }; - - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.new_stock, report.changes[0].kind); @@ -1305,7 +2069,7 @@ test "computeReport: drip::true existing lot with shares increase is drip_confir .{ .symbol = "FAGIX", .shares = 105, .open_date = Date.fromYmd(2026, 3, 1), .open_price = 11.00, .account = "Kelly IRA", .drip = true }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); try std.testing.expectEqual(@as(usize, 1), report.changes.len); try std.testing.expectEqual(ChangeKind.drip_confirmed, report.changes[0].kind); @@ -1330,7 +2094,7 @@ test "computeReport: per-account totals separate drip_confirmed from rollup" { .{ .symbol = "VBTLX", .shares = 1020, .open_date = Date.fromYmd(2026, 2, 1), .open_price = 9.79, .account = "AcctA" }, }; - const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18)); + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); const t = report.account_totals.get("AcctA") orelse { try std.testing.expect(false); diff --git a/src/commands/history.zig b/src/commands/history.zig index 66a997b..101e46d 100644 --- a/src/commands/history.zig +++ b/src/commands/history.zig @@ -65,7 +65,13 @@ pub const PortfolioOpts = struct { }; /// Parse the arg list for portfolio-mode flags. Pure function — no IO. +/// +/// `--since` and `--until` accept the same grammar as other commands: +/// `YYYY-MM-DD` or a relative shortcut like `1W`, `1M`, `1Q`, `1Y`. +/// Relative forms resolve against today (from the system clock) at +/// call time; pass explicit ISO dates for test determinism. pub fn parsePortfolioOpts(args: []const []const u8) Error!PortfolioOpts { + const today = fmt.todayDate(); var opts: PortfolioOpts = .{}; var i: usize = 0; while (i < args.len) : (i += 1) { @@ -73,11 +79,11 @@ pub fn parsePortfolioOpts(args: []const []const u8) Error!PortfolioOpts { if (std.mem.eql(u8, a, "--since")) { i += 1; if (i >= args.len) return error.MissingFlagValue; - opts.since = Date.parse(args[i]) catch return error.InvalidFlagValue; + opts.since = cli.parseRequiredDate(args[i], today) catch return error.InvalidFlagValue; } else if (std.mem.eql(u8, a, "--until")) { i += 1; if (i >= args.len) return error.MissingFlagValue; - opts.until = Date.parse(args[i]) catch return error.InvalidFlagValue; + opts.until = cli.parseRequiredDate(args[i], today) catch return error.InvalidFlagValue; } else if (std.mem.eql(u8, a, "--metric")) { i += 1; if (i >= args.len) return error.MissingFlagValue; diff --git a/src/commands/projections.zig b/src/commands/projections.zig index c5eb21e..b074e7b 100644 --- a/src/commands/projections.zig +++ b/src/commands/projections.zig @@ -302,11 +302,12 @@ pub fn run( /// Resolve the user's requested as-of date against the history directory. /// -/// Thin adapter over `history.resolveSnapshotDate` — the shared pure -/// resolver owns exact-then-fallback logic. This wrapper maps the -/// error set to `error.NoSnapshot` and surfaces user-visible stderr -/// messages (including the "Earliest available (later than requested)" -/// hint, which the TUI doesn't need). +/// Thin adapter over `cli.resolveSnapshotOrExplain` — the shared CLI +/// helper owns exact-then-fallback resolution and the stderr +/// messaging. This wrapper just maps the error set to +/// `error.NoSnapshot` (projections-specific) and strips the `exact` +/// flag since projections doesn't surface that distinction in its +/// header (it just uses `actual` directly). /// /// Arena-allocates the intermediate `hist_dir` + filename strings; /// pass a short-lived arena as `va`. @@ -317,28 +318,8 @@ fn resolveAsOfSnapshot( ) !AsOfResolution { const hist_dir = try history.deriveHistoryDir(va, file_path); - const resolved = history.resolveSnapshotDate(va, hist_dir, requested) catch |err| switch (err) { - error.NoSnapshotAtOrBefore => { - var req_buf: [10]u8 = undefined; - const req_str = requested.format(&req_buf); - const msg = std.fmt.allocPrint(va, "No snapshot at or before {s}.\n", .{req_str}) catch "No snapshot at or before the requested date.\n"; - try cli.stderrPrint(msg); - // Second look at the nearest table for the "later available" - // hint. Cheap (filesystem scan, same dir). - const nearest = history.findNearestSnapshot(hist_dir, requested) catch { - try cli.stderrPrint("No snapshots in history/ — run `zfin snapshot` to create one.\n"); - return error.NoSnapshot; - }; - if (nearest.later) |later| { - var later_buf: [10]u8 = undefined; - const later_str = later.format(&later_buf); - const later_msg = std.fmt.allocPrint(va, "Earliest available: {s} (later than requested).\n", .{later_str}) catch "A later snapshot exists but was not used.\n"; - try cli.stderrPrint(later_msg); - } else { - try cli.stderrPrint("No snapshots in history/ — run `zfin snapshot` to create one.\n"); - } - return error.NoSnapshot; - }, + const resolved = cli.resolveSnapshotOrExplain(va, hist_dir, requested) catch |err| switch (err) { + error.NoSnapshotAtOrBefore => return error.NoSnapshot, else => |e| { try cli.stderrPrint("Error resolving snapshot: "); try cli.stderrPrint(@errorName(e)); @@ -380,7 +361,7 @@ fn writeCell(out: *std.Io.Writer, color: bool, cell: view.ReturnCell, width: usi const testing = std.testing; const snapshot_model = @import("../models/snapshot.zig"); -const snapshot_cmd = @import("snapshot.zig"); +const snapshot = @import("snapshot.zig"); fn makeTestSvc() zfin.DataService { const config = zfin.Config{ .cache_dir = "/tmp" }; @@ -428,7 +409,7 @@ fn writeFixtureSnapshot( .accounts = &.{}, .lots = @constCast(&lots), }; - const rendered = try snapshot_cmd.renderSnapshot(allocator, snap); + const rendered = try snapshot.renderSnapshot(allocator, snap); defer allocator.free(rendered); try dir.writeFile(.{ .sub_path = filename, .data = rendered }); } diff --git a/src/commands/snapshot.zig b/src/commands/snapshot.zig index 0c5b2de..5927d94 100644 --- a/src/commands/snapshot.zig +++ b/src/commands/snapshot.zig @@ -97,12 +97,11 @@ pub fn run( } else if (std.mem.eql(u8, a, "--as-of")) { i += 1; if (i >= args.len) { - try cli.stderrPrint("Error: --as-of requires a date (YYYY-MM-DD)\n"); + try cli.stderrPrint("Error: --as-of requires a date (YYYY-MM-DD or shortcut like 1W/1M/1Q/1Y)\n"); return error.UnexpectedArg; } - as_of_override = Date.parse(args[i]) catch { - try cli.stderrPrint("Error: --as-of: invalid date (expected YYYY-MM-DD)\n"); - return error.UnexpectedArg; + as_of_override = cli.parseRequiredDateOrStderr(args[i], cli.fmt.todayDate(), "--as-of") catch |err| switch (err) { + error.InvalidDate => return error.UnexpectedArg, }; } else { try cli.stderrPrint("Error: unknown argument to 'snapshot': "); diff --git a/src/git.zig b/src/git.zig index 8760b57..3b62914 100644 --- a/src/git.zig +++ b/src/git.zig @@ -331,10 +331,16 @@ pub fn commitAtOrBeforeDate( rel_path: []const u8, date_iso: []const u8, ) Error!?[]const u8 { - // `git log --until=DATE` uses the end of the given date as the - // inclusive upper bound. Adding a "T23:59:59" suffix isn't - // necessary — git already interprets bare dates as "end of day". - const until_arg = try std.fmt.allocPrint(allocator, "--until={s}", .{date_iso}); + // `git log --until=DATE` with a bare YYYY-MM-DD uses the *current + // time-of-day* applied to DATE as the cutoff — NOT end of day as + // intuition suggests. That means at 10:40am today, `--until=X` + // excludes any commits on X made after 10:40am, which causes + // day-of-review windows to randomly include or exclude commits + // depending on when the command is run. Explicitly pin the cutoff + // to 23:59:59 local so "since 2026-04-25" always means "include + // all commits on 2026-04-25, regardless of what time I'm + // looking". + const until_arg = try std.fmt.allocPrint(allocator, "--until={s} 23:59:59", .{date_iso}); defer allocator.free(until_arg); const result = std.process.Child.run(.{ diff --git a/src/views/compare.zig b/src/views/compare.zig index cb64b56..aca9667 100644 --- a/src/views/compare.zig +++ b/src/views/compare.zig @@ -137,6 +137,14 @@ pub const CompareView = struct { /// Symbols present in "then" but not "now" — position closed /// between the two dates. Never rendered as rows; shown as a count. removed_count: usize, + /// Number of held-throughout symbols with `pct_change > flat_threshold`. + /// Intended for the per-symbol summary footer ("21 gainers, 5 losers"). + gainer_count: usize = 0, + /// Number of held-throughout symbols with `pct_change < -flat_threshold`. + loser_count: usize = 0, + /// Number of held-throughout symbols with `|pct_change| <= flat_threshold`. + /// `gainer_count + loser_count + flat_count == held_count`. + flat_count: usize = 0, /// Optional contributions-vs-gains breakdown of `liquid.delta`. /// Populated by the CLI from `computeAttribution` when a git repo /// is available; always null in unit-tested / TUI flows. @@ -147,6 +155,14 @@ pub const CompareView = struct { } }; +/// Threshold under which a pct_change is considered flat for the +/// gainer/loser summary footer. `0.0001 == 0.01%`. Chosen so penny- +/// level rounding noise on high-priced positions (e.g. a $500 stock +/// moving one cent is 0.002%) stays out of the "gainers" bucket while +/// anything visibly colored as positive/negative in the per-symbol +/// table crosses the threshold. +pub const flat_threshold: f64 = 0.0001; + /// One entry in a holdings snapshot — total shares held of `symbol` and /// the per-share price at that moment. Caller-populated; the view model /// doesn't know or care where the numbers came from. @@ -261,6 +277,23 @@ pub fn buildCompareView( } }.lt); + // Bucket held-throughout rows into gainers / losers / flat using + // `flat_threshold` so that cent-rounding noise on a high-priced + // position doesn't get counted as a win or a loss. Computed after + // the sort purely for locality — buckets are independent of order. + var gainers: usize = 0; + var losers: usize = 0; + var flats: usize = 0; + for (changes.items) |c| { + if (c.pct_change > flat_threshold) { + gainers += 1; + } else if (c.pct_change < -flat_threshold) { + losers += 1; + } else { + flats += 1; + } + } + const items = try changes.toOwnedSlice(allocator); return .{ .then_date = then_date, @@ -272,6 +305,9 @@ pub fn buildCompareView( .held_count = items.len, .added_count = added, .removed_count = removed, + .gainer_count = gainers, + .loser_count = losers, + .flat_count = flats, }; } @@ -487,6 +523,73 @@ test "buildCompareView: sort strictly descending across many symbols" { try testing.expectEqualStrings("E", view.symbols[2].symbol); try testing.expectEqualStrings("D", view.symbols[3].symbol); try testing.expectEqualStrings("B", view.symbols[4].symbol); + + // Gainer/loser buckets: C/A/E > flat_threshold, D/B < -flat_threshold + try testing.expectEqual(@as(usize, 3), view.gainer_count); + try testing.expectEqual(@as(usize, 2), view.loser_count); + try testing.expectEqual(@as(usize, 0), view.flat_count); +} + +test "buildCompareView: gainer/loser/flat buckets with near-zero moves" { + var then_map: HoldingMap = .init(testing.allocator); + defer then_map.deinit(); + var now_map: HoldingMap = .init(testing.allocator); + defer now_map.deinit(); + + // Clear gainer + try then_map.put("UP", .{ .shares = 1, .price = 100.0 }); + try now_map.put("UP", .{ .shares = 1, .price = 101.0 }); // +1% + // Clear loser + try then_map.put("DN", .{ .shares = 1, .price = 100.0 }); + try now_map.put("DN", .{ .shares = 1, .price = 98.0 }); // -2% + // Exactly flat + try then_map.put("FLAT", .{ .shares = 1, .price = 100.0 }); + try now_map.put("FLAT", .{ .shares = 1, .price = 100.0 }); // 0% + // Rounding-noise positive (below flat_threshold of 0.01%) + try then_map.put("NOISE", .{ .shares = 1, .price = 1000.0 }); + try now_map.put("NOISE", .{ .shares = 1, .price = 1000.05 }); // +0.005% + + var view = try buildCompareView( + testing.allocator, + Date.fromYmd(2026, 1, 1), + Date.fromYmd(2026, 2, 1), + false, + 4000.0, + 4001.0, + &then_map, + &now_map, + ); + defer view.deinit(testing.allocator); + + try testing.expectEqual(@as(usize, 4), view.held_count); + try testing.expectEqual(@as(usize, 1), view.gainer_count); + try testing.expectEqual(@as(usize, 1), view.loser_count); + try testing.expectEqual(@as(usize, 2), view.flat_count); + // Sanity: buckets sum to held_count. + try testing.expectEqual(view.held_count, view.gainer_count + view.loser_count + view.flat_count); +} + +test "buildCompareView: zero held-throughout yields zero counts for all three buckets" { + var then_map: HoldingMap = .init(testing.allocator); + defer then_map.deinit(); + var now_map: HoldingMap = .init(testing.allocator); + defer now_map.deinit(); + + var view = try buildCompareView( + testing.allocator, + Date.fromYmd(2026, 1, 1), + Date.fromYmd(2026, 1, 1), + true, + 0.0, + 0.0, + &then_map, + &now_map, + ); + defer view.deinit(testing.allocator); + + try testing.expectEqual(@as(usize, 0), view.gainer_count); + try testing.expectEqual(@as(usize, 0), view.loser_count); + try testing.expectEqual(@as(usize, 0), view.flat_count); } // ── Layout constants + row-cell builders ─────────────────────