diff --git a/TODO.md b/TODO.md index 2a46ac8..be5c8b4 100644 --- a/TODO.md +++ b/TODO.md @@ -215,36 +215,6 @@ hygiene without inheriting the reconciliation surface. Pure internal refactor; no user-visible change. -## Audit: reconcile accounts present in the portfolio but absent from the export — priority MEDIUM - -`compareAccounts` (and `compareSchwabSummary`) iterate over the -accounts found in the *brokerage export*, then look up the matching -portfolio account. The two directions are asymmetric: - -- **Export row → no portfolio account:** handled. The - `portfolio_acct_name == null` branch surfaces it as - "unmapped — add account_number to accounts.srf" and flags a - discrepancy. -- **Portfolio account → not in the export:** *silent gap.* An - account that exists in `accounts.srf` / has lots in - `portfolio.srf` but has no corresponding account in the CSV is - never iterated, so the reconciler says nothing. If you forget to - include an account in the download, or a brokerage drops it from - the export, audit can't tell you "you hold account X that wasn't - in this file — stale, or just not exported?" - -Fix sketch: after the per-export-account loop, walk the -`account_map` entries for the institution being reconciled and, for -any whose portfolio account holds open lots as-of but never appeared -in the export, emit a "portfolio account not found in export" notice. -Gate it to the institution under audit (don't flag Schwab accounts -when reconciling a Fidelity export). Decide whether a zero-balance / -fully-closed account should be suppressed. - -Found while debugging a BrokerageLink cash reconciliation — that -account *was* in the export, so this gap wasn't the culprit, but the -asymmetry is real and worth closing. - ## Refactor: trim `src/format.zig` once Money / Date have absorbed their helpers — priority LOW `src/format.zig` is still a ~1700-line grab-bag, but the money- and diff --git a/src/commands/audit.zig b/src/commands/audit.zig index d6c9dfb..80dd3fa 100644 --- a/src/commands/audit.zig +++ b/src/commands/audit.zig @@ -270,6 +270,20 @@ pub const AccountComparison = struct { has_discrepancies: bool, }; +/// A portfolio account that maps to the institution under audit and +/// still holds open lots as-of, but whose account number never +/// appeared in the brokerage export. Surfaced as an advisory so an +/// account that was dropped from the download (or simply never +/// exported) doesn't reconcile silently. See `findAbsentAccounts`. +pub const AbsentAccount = struct { + /// Portfolio account name. Borrows from the account map. + account_name: []const u8, + /// Mapped account number. Borrows from the account map. + account_number: []const u8, + /// Current value of the account's open holdings as-of, for context. + portfolio_total: f64, +}; + /// Consolidate broker rows that share a symbol within the same /// account into a single position. Some brokers split a single /// stock holding into separate "Cash" and "Margin" rows for the @@ -638,6 +652,81 @@ pub fn compareAccounts( return results.toOwnedSlice(allocator); } +// ── Portfolio accounts absent from the export ──────────────── + +/// Collect the account numbers carried by a set of comparison +/// results. Monomorphized over the known result types that expose an +/// `account_number` field (`AccountComparison`, `SchwabAccountComparison`) +/// so the same membership input feeds `findAbsentAccounts` regardless +/// of which reconciler produced the results. Caller owns the slice; +/// the elements borrow from `results`. +fn presentNumbers(allocator: std.mem.Allocator, comptime T: type, results: []const T) ![][]const u8 { + var nums: std.ArrayList([]const u8) = .empty; + errdefer nums.deinit(allocator); + for (results) |r| try nums.append(allocator, r.account_number); + return nums.toOwnedSlice(allocator); +} + +/// Find portfolio accounts mapped to `institution` that still hold +/// open lots as-of but whose account number is absent from +/// `present_numbers` (the numbers that appeared in the brokerage +/// export). +/// +/// This closes a long-standing asymmetry: `compareAccounts` and +/// `compareSchwabSummary` walk export → portfolio only, so an account +/// you hold that the export dropped (forgotten in the download, or +/// silently removed by the broker) reconciles to nothing and the +/// audit says nothing. Walking the other direction here surfaces it. +/// +/// Gating: only entries whose `institution` matches are considered, so +/// a Fidelity export never flags Schwab accounts. Suppression: +/// fully-closed / zero-balance accounts (no open lots as-of) are +/// skipped — a dropped account is only actionable if you still hold +/// something in it. Entries with no `account_number` are skipped too: +/// without a number they can't be matched to an export row anyway. +/// +/// Caller owns the returned slice. The string fields borrow from +/// `account_map`, which must outlive the result. +pub fn findAbsentAccounts( + allocator: std.mem.Allocator, + portfolio: zfin.Portfolio, + account_map: analysis.AccountMap, + institution: []const u8, + present_numbers: []const []const u8, + prices: std.StringHashMap(f64), + as_of: Date, +) ![]AbsentAccount { + var results: std.ArrayList(AbsentAccount) = .empty; + errdefer results.deinit(allocator); + + for (account_map.entries) |e| { + const inst = e.institution orelse continue; + if (!std.mem.eql(u8, inst, institution)) continue; + const num = e.account_number orelse continue; + + // Present in the export? The export → portfolio pass covered it. + var present = false; + for (present_numbers) |pn| { + if (std.mem.eql(u8, pn, num)) { + present = true; + break; + } + } + if (present) continue; + + // Nothing held as-of → nothing to reconcile. Suppress. + if (!portfolio.hasOpenLotsForAccount(as_of, e.account)) continue; + + try results.append(allocator, .{ + .account_name = e.account, + .account_number = num, + .portfolio_total = portfolio.totalForAccount(as_of, allocator, e.account, prices), + }); + } + + return results.toOwnedSlice(allocator); +} + // ── Ratio suggestions ──────────────────────────────────────── /// After displaying audit results, check for price_ratio positions where @@ -1018,6 +1107,24 @@ fn displayResults(results: []const AccountComparison, color: bool, out: *std.Io. try out.print("\n", .{}); } +/// Render the "portfolio accounts not found in this export" advisory. +/// Silent when `absent` is empty, so it composes cleanly after both +/// the verbose audit table and the compact "no discrepancies" line. +fn displayAbsentAccounts(absent: []const AbsentAccount, color: bool, out: *std.Io.Writer) !void { + if (absent.len == 0) return; + + try out.print("\n", .{}); + try cli.printFg(out, color, cli.CLR_WARNING, " Portfolio accounts not found in this export", .{}); + try cli.printFg(out, color, cli.CLR_MUTED, " (stale, or not exported?)\n", .{}); + for (absent) |a| { + try out.print(" ", .{}); + try cli.printFg(out, color, cli.CLR_WARNING, "{s}", .{a.account_name}); + try cli.printFg(out, color, cli.CLR_MUTED, " (#{s})", .{a.account_number}); + try out.print(" {f}\n", .{Money.from(a.portfolio_total)}); + } + try out.print("\n", .{}); +} + // ── Hygiene check (flagless audit) ────────────────────────── /// Constants for hygiene check behavior. Kept as named constants for @@ -1863,6 +1970,11 @@ fn runHygieneCheck( const results = compareSchwabSummary(allocator, portfolio, schwab_accounts, account_map, prices, as_of) catch continue; defer allocator.free(results); + const present = try presentNumbers(allocator, SchwabAccountComparison, results); + defer allocator.free(present); + const absent = try findAbsentAccounts(allocator, portfolio, account_map, "schwab", present, prices, as_of); + defer allocator.free(absent); + if (verbose or hasSchwabDiscrepancies(results)) { try out.print("\n", .{}); try displaySchwabResults(results, color, out); @@ -1878,6 +1990,7 @@ fn runHygieneCheck( // non-zero delta that still deserves a nudge. try displaySchwabSummaryRatioSuggestions(results, portfolio, prices, account_map, color, out); } + try displayAbsentAccounts(absent, color, out); }, .fidelity_csv => { const brokerage_positions = parseFidelityCsv(allocator, file_data) catch continue; @@ -1889,6 +2002,11 @@ fn runHygieneCheck( allocator.free(results); } + const present = try presentNumbers(allocator, AccountComparison, results); + defer allocator.free(present); + const absent = try findAbsentAccounts(allocator, portfolio, account_map, "fidelity", present, prices, as_of); + defer allocator.free(absent); + if (verbose or hasAccountDiscrepancies(results)) { try out.print("\n", .{}); try displayResults(results, color, out); @@ -1898,6 +2016,7 @@ fn runHygieneCheck( // Always show ratio suggestions even in compact mode try displayRatioSuggestions(results, portfolio, prices, account_map, color, out); } + try displayAbsentAccounts(absent, color, out); }, .schwab_csv => { const parsed = parseSchwabCsv(allocator, file_data) catch continue; @@ -1909,6 +2028,11 @@ fn runHygieneCheck( allocator.free(results); } + const present = try presentNumbers(allocator, AccountComparison, results); + defer allocator.free(present); + const absent = try findAbsentAccounts(allocator, portfolio, account_map, "schwab", present, prices, as_of); + defer allocator.free(absent); + if (verbose or hasAccountDiscrepancies(results)) { try out.print("\n", .{}); try displayResults(results, color, out); @@ -1917,6 +2041,7 @@ fn runHygieneCheck( try cli.printFg(out, color, cli.CLR_POSITIVE, " schwab: {d} accounts, no discrepancies\n", .{results.len}); try displayRatioSuggestions(results, portfolio, prices, account_map, color, out); } + try displayAbsentAccounts(absent, color, out); }, } } @@ -2140,6 +2265,12 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { try displaySchwabResults(results, color, out); try displaySchwabSummaryRatioSuggestions(results, portfolio, prices, account_map, color, out); + + const present = try presentNumbers(allocator, SchwabAccountComparison, results); + defer allocator.free(present); + const absent = try findAbsentAccounts(allocator, portfolio, account_map, "schwab", present, prices, as_of); + defer allocator.free(absent); + try displayAbsentAccounts(absent, color, out); } // Fidelity CSV @@ -2166,6 +2297,12 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { try displayResults(results, color, out); try displayRatioSuggestions(results, portfolio, prices, account_map, color, out); + + const present = try presentNumbers(allocator, AccountComparison, results); + defer allocator.free(present); + const absent = try findAbsentAccounts(allocator, portfolio, account_map, "fidelity", present, prices, as_of); + defer allocator.free(absent); + try displayAbsentAccounts(absent, color, out); } // Schwab per-account CSV @@ -2192,6 +2329,12 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { try displayResults(results, color, out); try displayRatioSuggestions(results, portfolio, prices, account_map, color, out); + + const present = try presentNumbers(allocator, AccountComparison, results); + defer allocator.free(present); + const absent = try findAbsentAccounts(allocator, portfolio, account_map, "schwab", present, prices, as_of); + defer allocator.free(absent); + try displayAbsentAccounts(absent, color, out); } } @@ -2578,6 +2721,143 @@ test "compareAccounts: sub-dollar cash drift is flagged (cash matches to the pen try std.testing.expect(found_cash); } +test "presentNumbers: collects account_number from each result" { + const allocator = std.testing.allocator; + + const results = [_]AccountComparison{ + .{ .account_name = "Sample IRA", .brokerage_name = "Fid", .account_number = "1234", .comparisons = &.{}, .portfolio_total = 0, .brokerage_total = 0, .total_delta = 0, .option_value_delta = 0, .has_discrepancies = false }, + .{ .account_name = "Sample Brokerage", .brokerage_name = "Fid", .account_number = "5678", .comparisons = &.{}, .portfolio_total = 0, .brokerage_total = 0, .total_delta = 0, .option_value_delta = 0, .has_discrepancies = false }, + }; + + const nums = try presentNumbers(allocator, AccountComparison, &results); + defer allocator.free(nums); + + try std.testing.expectEqual(@as(usize, 2), nums.len); + try std.testing.expectEqualStrings("1234", nums[0]); + try std.testing.expectEqualStrings("5678", nums[1]); +} + +test "findAbsentAccounts: flags held account missing from export; honors gating + closed-account suppression" { + const allocator = std.testing.allocator; + const as_of = Date.fromYmd(2026, 6, 19); + + var lots = [_]portfolio_mod.Lot{ + // fidelity #1234 → held, absent from export → SHOULD flag. + .{ .symbol = "VTI", .shares = 10, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 200.0, .account = "Sample IRA" }, + // fidelity #5678 → held, present in export → handled by main pass. + .{ .symbol = "AAPL", .shares = 5, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 150.0, .account = "Sample Brokerage" }, + // fidelity #3456 → only a closed lot, absent → suppressed. + .{ .symbol = "MSFT", .shares = 5, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 300.0, .close_date = Date.fromYmd(2025, 1, 1), .close_price = 350.0, .account = "Sample Roth" }, + // schwab #9012 → held, absent, but wrong institution for a Fidelity audit. + .{ .symbol = "NVDA", .shares = 2, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 400.0, .account = "Schwab Trust" }, + }; + const portfolio = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator }; + + var entries = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample IRA", .tax_type = .traditional, .institution = "fidelity", .account_number = "1234" }, + .{ .account = "Sample Brokerage", .tax_type = .taxable, .institution = "fidelity", .account_number = "5678" }, + .{ .account = "Sample Roth", .tax_type = .roth, .institution = "fidelity", .account_number = "3456" }, + .{ .account = "Schwab Trust", .tax_type = .taxable, .institution = "schwab", .account_number = "9012" }, + // institution set but no account number → can't match an export row → skipped. + .{ .account = "Sample HSA", .tax_type = .hsa, .institution = "fidelity", .account_number = null }, + }; + const acct_map = analysis.AccountMap{ .entries = &entries, .allocator = allocator }; + + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("VTI", 210.0); + + // The Fidelity export contained only account #5678. + const present = [_][]const u8{"5678"}; + + const absent = try findAbsentAccounts(allocator, portfolio, acct_map, "fidelity", &present, prices, as_of); + defer allocator.free(absent); + + try std.testing.expectEqual(@as(usize, 1), absent.len); + try std.testing.expectEqualStrings("Sample IRA", absent[0].account_name); + try std.testing.expectEqualStrings("1234", absent[0].account_number); + try std.testing.expectApproxEqAbs(@as(f64, 2100.0), absent[0].portfolio_total, 0.01); +} + +test "findAbsentAccounts: gating flags only the audited institution" { + const allocator = std.testing.allocator; + const as_of = Date.fromYmd(2026, 6, 19); + + var lots = [_]portfolio_mod.Lot{ + .{ .symbol = "VTI", .shares = 10, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 200.0, .account = "Sample IRA" }, + .{ .symbol = "NVDA", .shares = 2, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 400.0, .account = "Schwab Trust" }, + }; + const portfolio = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator }; + + var entries = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample IRA", .tax_type = .traditional, .institution = "fidelity", .account_number = "1234" }, + .{ .account = "Schwab Trust", .tax_type = .taxable, .institution = "schwab", .account_number = "9012" }, + }; + const acct_map = analysis.AccountMap{ .entries = &entries, .allocator = allocator }; + + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + // Auditing a Schwab export that matched no known account number: + // only the Schwab account surfaces; the Fidelity account is gated out. + const present = [_][]const u8{}; + const absent = try findAbsentAccounts(allocator, portfolio, acct_map, "schwab", &present, prices, as_of); + defer allocator.free(absent); + + try std.testing.expectEqual(@as(usize, 1), absent.len); + try std.testing.expectEqualStrings("Schwab Trust", absent[0].account_name); + try std.testing.expectEqualStrings("9012", absent[0].account_number); +} + +test "findAbsentAccounts: no absent accounts when export covers every held account" { + const allocator = std.testing.allocator; + const as_of = Date.fromYmd(2026, 6, 19); + + var lots = [_]portfolio_mod.Lot{ + .{ .symbol = "VTI", .shares = 10, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 200.0, .account = "Sample IRA" }, + }; + const portfolio = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator }; + + var entries = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample IRA", .tax_type = .traditional, .institution = "fidelity", .account_number = "1234" }, + }; + const acct_map = analysis.AccountMap{ .entries = &entries, .allocator = allocator }; + + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const present = [_][]const u8{"1234"}; + const absent = try findAbsentAccounts(allocator, portfolio, acct_map, "fidelity", &present, prices, as_of); + defer allocator.free(absent); + + try std.testing.expectEqual(@as(usize, 0), absent.len); +} + +test "displayAbsentAccounts: silent when empty, renders names + totals otherwise" { + var buf: [1024]u8 = undefined; + + // Empty → no output. + { + var w = std.Io.Writer.fixed(&buf); + try displayAbsentAccounts(&.{}, false, &w); + try std.testing.expectEqual(@as(usize, 0), w.buffered().len); + } + + // Non-empty → names, numbers, and a money total appear. + { + var w = std.Io.Writer.fixed(&buf); + const absent = [_]AbsentAccount{ + .{ .account_name = "Sample IRA", .account_number = "1234", .portfolio_total = 2100.0 }, + }; + try displayAbsentAccounts(&absent, false, &w); + const out = w.buffered(); + try std.testing.expect(std.mem.indexOf(u8, out, "not found in this export") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "Sample IRA") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "#1234") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "$2,100") != null); + } +} + test "detectBrokerFileKind: fidelity csv" { const fidelity_header = "Account Number,Account Name,Symbol,Description"; try std.testing.expectEqual(BrokerFileKind.fidelity_csv, detectBrokerFileKind(fidelity_header).?); diff --git a/src/models/portfolio.zig b/src/models/portfolio.zig index b61c2ef..8afb7ee 100644 --- a/src/models/portfolio.zig +++ b/src/models/portfolio.zig @@ -569,6 +569,24 @@ pub const Portfolio = struct { return total; } + /// True if `account_name` holds at least one open lot as-of — any + /// real holding type (stock, cash, CD, option). Watchlist entries + /// (`.watch`, share count zero) don't count: they're not held. + /// + /// Used by the audit reconciler to decide whether a portfolio + /// account that's missing from a brokerage export is worth + /// flagging. A fully-closed / zero-balance account has nothing left + /// to reconcile, so it's suppressed. + pub fn hasOpenLotsForAccount(self: Portfolio, as_of: Date, account_name: []const u8) bool { + for (self.lots) |lot| { + if (lot.security_type == .watch) continue; + const lot_acct = lot.account orelse continue; + if (!std.mem.eql(u8, lot_acct, account_name)) continue; + if (lot.isOpen(as_of)) return true; + } + return false; + } + /// Total value of non-stock holdings (cash, CDs, options) for a single account. /// Only includes open lots (respects close_date and maturity_date). pub fn nonStockValueForAccount(self: Portfolio, as_of: Date, account_name: []const u8) f64 { @@ -1337,6 +1355,34 @@ test "nonStockValueForAccount" { try std.testing.expectApproxEqAbs(@as(f64, 1000.0), ns_other, 0.01); } +test "hasOpenLotsForAccount: open stock, cash; closed and watch excluded" { + const allocator = std.testing.allocator; + const as_of = Date.fromYmd(2026, 5, 8); + + var lots = [_]Lot{ + // Open stock in Sample IRA. + .{ .symbol = "AAPL", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 150.0, .account = "Sample IRA" }, + // Sample Brokerage holds only a closed stock lot. + .{ .symbol = "MSFT", .shares = 50, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 300.0, .close_date = Date.fromYmd(2025, 1, 1), .close_price = 350.0, .account = "Sample Brokerage" }, + // Sample Roth holds only an open cash lot. + .{ .symbol = "", .shares = 2000, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Roth" }, + // Sample HSA holds only a watchlist entry (not a real holding). + .{ .symbol = "NVDA", .shares = 0, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 0, .security_type = .watch, .account = "Sample HSA" }, + }; + const portfolio = Portfolio{ .lots = &lots, .allocator = allocator }; + + try std.testing.expect(portfolio.hasOpenLotsForAccount(as_of, "Sample IRA")); + try std.testing.expect(portfolio.hasOpenLotsForAccount(as_of, "Sample Roth")); + // Closed-only account → no open lots. + try std.testing.expect(!portfolio.hasOpenLotsForAccount(as_of, "Sample Brokerage")); + // Watchlist-only account → not held. + try std.testing.expect(!portfolio.hasOpenLotsForAccount(as_of, "Sample HSA")); + // Account with no lots at all. + try std.testing.expect(!portfolio.hasOpenLotsForAccount(as_of, "Sample Trust")); + // Before the open date the stock isn't held yet. + try std.testing.expect(!portfolio.hasOpenLotsForAccount(Date.fromYmd(2023, 1, 1), "Sample IRA")); +} + test "totalForAccount" { const allocator = std.testing.allocator; const future = Date.fromYmd(2099, 12, 31);