audit: reconcile against all paths, not just anchor

This commit is contained in:
Emil Lerch 2026-10-14 07:55:55 -07:00
parent a966eed026
commit 4a33f1267a
Signed by: lobo
GPG key ID: A7B62D657EF764F8
2 changed files with 139 additions and 53 deletions

View file

@ -133,17 +133,15 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
const verbose = parsed.verbose;
const stale_days = parsed.stale_days;
// Flagless mode: run portfolio hygiene check. Mostly single-file
// semantics (git blame, commit SHAs, etc.), so resolve the anchor -
// but the large-lot check inside diffs portfolio content and needs
// the merged view, so hand it the whole glob too.
// Flagless mode: run the portfolio hygiene check, on the same load
// the reconciliation modes below use - the union of every portfolio
// file, split-adjusted - so both modes reconcile the same lots.
// (Hygiene still reads the anchor file on its own for the checks
// that diff it against its git history.)
if (fidelity_csv == null and schwab_csv == null and !schwab_summary and wells_fargo_xls == null) {
const pf = ctx.resolvePortfolioPath();
defer pf.deinit(allocator);
var all = ctx.resolvePortfolioPaths() catch null;
defer if (all) |*p| p.deinit();
const paths: []const []const u8 = if (all) |p| p.paths else &.{pf.path};
return hygiene.runHygieneCheck(io, allocator, ctx.environ_map, svc, pf.path, paths, stale_days, verbose, as_of, now_s, color, ctx.globals.refresh_policy, out);
var live = cli.loadPortfolio(ctx, as_of) orelse return;
defer live.deinit(allocator);
return hygiene.runHygieneCheck(io, allocator, ctx.environ_map, svc, &live, stale_days, verbose, as_of, now_s, color, ctx.globals.refresh_policy, out);
}
// Reconciliation modes (--fidelity / --schwab / --schwab-summary /

View file

@ -30,6 +30,7 @@ const git = @import("../../git.zig");
const test_git = @import("../../testutil/git.zig");
const common = @import("common.zig");
const portfolio_loader = @import("../../portfolio_loader.zig");
const fidelity = @import("../../analytics/reconcile/fidelity.zig");
const wells_fargo = @import("../../analytics/reconcile/wells_fargo.zig");
const wf_parser = @import("../../brokerage/wells_fargo.zig");
@ -777,17 +778,25 @@ fn reportAccountExport(
}
/// Run the flagless portfolio hygiene check.
///
/// `live` is the portfolio as every other command sees it -
/// `cli.loadPortfolio`'s union of the whole `portfolio*.srf` glob, split
/// adjustment applied - and is what the content checks read: stale
/// manual prices, reconciliation, the large-lot check and unhandled
/// splits. Reconciliation used to read `portfolio_path` alone, so a
/// holding kept in a sibling file showed as brokerage-only here while
/// `zfin audit --schwab FILE` reconciled it cleanly, and split-adjusted
/// share counts differed between the two modes.
///
/// `portfolio_path` (the anchor, `live.anchor()`) is still read on its
/// own for the checks that diff that file against its own git history:
/// manual-price edits since HEAD and per-account update cadence.
pub fn runHygieneCheck(
io: std.Io,
allocator: std.mem.Allocator,
env: *const std.process.Environ.Map,
svc: *zfin.DataService,
portfolio_path: []const u8,
/// Every file in the `portfolio*.srf` glob. Only the large-lot
/// check needs this: it diffs portfolio CONTENT and so must see
/// the merged view. The rest of the hygiene report is deliberately
/// single-file (git blame, commit SHAs) and uses `portfolio_path`.
portfolio_paths: []const []const u8,
live: *const cli.LoadedPortfolio,
stale_days: u32,
verbose: bool,
as_of: Date,
@ -796,15 +805,22 @@ pub fn runHygieneCheck(
refresh: framework.RefreshPolicy,
out: *std.Io.Writer,
) !void {
// Load portfolio
const pf_data = std.Io.Dir.cwd().readFileAlloc(io, portfolio_path, allocator, .limited(10 * 1024 * 1024)) catch {
cli.stderrPrint(io, "Error: Cannot read portfolio file\n");
const portfolio_path = live.anchor();
const portfolio_paths = live.paths;
// The merged, split-adjusted view: what the brokerage is compared to.
const merged = live.portfolio;
// The anchor file alone, for the git-history diffs below.
const pf_data = std.Io.Dir.cwd().readFileAlloc(io, portfolio_path, allocator, .limited(10 * 1024 * 1024)) catch |err| {
var buf: [512]u8 = undefined;
cli.stderrPrint(io, std.fmt.bufPrint(&buf, "Error: Cannot read portfolio file {s}: {s}\n", .{ portfolio_path, @errorName(err) }) catch "Error: Cannot read portfolio file\n");
return;
};
defer allocator.free(pf_data);
var portfolio = zfin.cache.deserializePortfolio(allocator, pf_data) catch {
cli.stderrPrint(io, "Error: Cannot parse portfolio file\n");
var portfolio = zfin.cache.deserializePortfolio(allocator, pf_data) catch |err| {
var buf: [512]u8 = undefined;
cli.stderrPrint(io, std.fmt.bufPrint(&buf, "Error: Cannot parse portfolio file {s}: {s}\n", .{ portfolio_path, @errorName(err) }) catch "Error: Cannot parse portfolio file\n");
return;
};
defer portfolio.deinit();
@ -828,7 +844,7 @@ pub fn runHygieneCheck(
// CDs/cash/options are excluded - their `price` is a fixed face
// value, not an age-stale quote - as are closed lots.
{
var stale = try collectStaleManualPrices(allocator, portfolio, as_of, stale_days);
var stale = try collectStaleManualPrices(allocator, merged, as_of, stale_days);
defer stale.deinit(allocator);
std.mem.sort(StaleManualPrice, stale.items, {}, staleLessThan);
@ -1141,8 +1157,6 @@ pub fn runHygieneCheck(
// the candle cache and printed a second "Loaded N symbols" line - one
// that landed in the middle of the reconciliation table. Stays null
// when no export was discovered, and Section 6 loads its own.
var recon_syms: [][]const u8 = &.{};
defer allocator.free(recon_syms);
var recon_load: ?zfin.DataService.LoadAllResult = null;
defer if (recon_load) |*r| r.deinit();
@ -1159,15 +1173,14 @@ pub fn runHygieneCheck(
var prices = std.StringHashMap(f64).init(allocator);
defer prices.deinit();
{
recon_syms = try portfolio.stockSymbols(allocator);
if (recon_syms.len > 0) {
recon_load = cli.loadPortfolioPrices(io, svc, recon_syms, &.{}, refresh, color);
if (live.syms.len > 0) {
recon_load = cli.loadPortfolioPrices(io, svc, live.syms, &.{}, refresh, color);
var pit = recon_load.?.prices.iterator();
while (pit.next()) |entry| {
try prices.put(entry.key_ptr.*, entry.value_ptr.*);
}
}
for (portfolio.lots) |lot| {
for (merged.lots) |lot| {
if (lot.price) |p| {
if (!prices.contains(lot.priceSymbol())) {
try prices.put(lot.priceSymbol(), lot.effectivePrice(p, false));
@ -1205,7 +1218,7 @@ pub fn runHygieneCheck(
switch (f.kind) {
.schwab_summary => {
const results = schwab.reconcileSummary(allocator, portfolio, file_data, account_map, prices, as_of) catch |err| {
const results = schwab.reconcileSummary(allocator, merged, file_data, account_map, prices, as_of) catch |err| {
try cli.printFg(out, color, cli.CLR_WARNING, " {s}: detected as schwab summary but could not parse ({s}); skipped\n", .{ f.path, @errorName(err) });
continue;
};
@ -1227,13 +1240,13 @@ pub fn runHygieneCheck(
// Always show ratio suggestions, even in compact mode -
// direct-indexing drift may cause a non-zero delta that
// still deserves a nudge.
try schwab.displaySchwabSummaryRatioSuggestions(allocator, view.shown, portfolio, prices, account_map, as_of, color, out);
try schwab.displaySchwabSummaryRatioSuggestions(allocator, view.shown, merged, prices, account_map, as_of, color, out);
try common.displayUnmapped(schwab.SchwabAccountComparison, "schwab summary", view, verbose, color, out);
try accumulatePresent(allocator, &schwab_present, schwab.SchwabAccountComparison, results);
},
.fidelity_csv => {
const results = fidelity.reconcile(allocator, portfolio, file_data, account_map, prices, as_of) catch |err| {
const results = fidelity.reconcile(allocator, merged, file_data, account_map, prices, as_of) catch |err| {
try cli.printFg(out, color, cli.CLR_WARNING, " {s}: detected as fidelity CSV but could not parse ({s}); skipped\n", .{ f.path, @errorName(err) });
continue;
};
@ -1243,11 +1256,11 @@ pub fn runHygieneCheck(
}
// Fidelity's download covers every account in the login.
if (try reportAccountExport(allocator, "fidelity", results, .all_accounts, portfolio, prices, account_map, verbose, color, out))
if (try reportAccountExport(allocator, "fidelity", results, .all_accounts, merged, prices, account_map, verbose, color, out))
try accumulatePresent(allocator, &fidelity_present, common.AccountComparison, results);
},
.schwab_csv => {
const results = schwab.reconcileCsv(allocator, portfolio, file_data, account_map, prices, as_of) catch |err| {
const results = schwab.reconcileCsv(allocator, merged, file_data, account_map, prices, as_of) catch |err| {
try cli.printFg(out, color, cli.CLR_WARNING, " {s}: detected as schwab CSV but could not parse ({s}); skipped\n", .{ f.path, @errorName(err) });
continue;
};
@ -1256,11 +1269,11 @@ pub fn runHygieneCheck(
allocator.free(results);
}
if (try reportAccountExport(allocator, "schwab", results, schwab.csvScope(file_data), portfolio, prices, account_map, verbose, color, out))
if (try reportAccountExport(allocator, "schwab", results, schwab.csvScope(file_data), merged, prices, account_map, verbose, color, out))
try accumulatePresent(allocator, &schwab_present, common.AccountComparison, results);
},
.wells_fargo_xls => {
var rec = wells_fargo.reconcileXls(allocator, portfolio, file_data, account_map, prices, as_of) catch |err| switch (err) {
var rec = wells_fargo.reconcileXls(allocator, merged, file_data, account_map, prices, as_of) catch |err| switch (err) {
error.OutOfMemory => return err,
else => |e| {
try cli.printFg(out, color, cli.CLR_WARNING, " {s}: detected as wells fargo export but could not parse ({s}); skipped\n", .{ f.path, @errorName(e) });
@ -1270,7 +1283,7 @@ pub fn runHygieneCheck(
defer rec.deinit(allocator);
// One WF download covers a whole household.
if (try reportAccountExport(allocator, "wells fargo", rec.results, .all_accounts, portfolio, prices, account_map, verbose, color, out))
if (try reportAccountExport(allocator, "wells fargo", rec.results, .all_accounts, merged, prices, account_map, verbose, color, out))
try accumulatePresent(allocator, &wells_fargo_present, common.AccountComparison, rec.results);
},
}
@ -1282,17 +1295,17 @@ pub fn runHygieneCheck(
// (see `displayAbsentAccounts`), so an account covered by any
// sibling CSV or the summary no longer surfaces here.
if (fidelity_present.items.len > 0) {
const absent = try common.findAbsentAccounts(allocator, portfolio, account_map, "fidelity", fidelity_present.items, prices, as_of);
const absent = try common.findAbsentAccounts(allocator, merged, account_map, "fidelity", fidelity_present.items, prices, as_of);
defer allocator.free(absent);
try common.displayAbsentAccounts(absent, color, "any export", out);
}
if (schwab_present.items.len > 0) {
const absent = try common.findAbsentAccounts(allocator, portfolio, account_map, "schwab", schwab_present.items, prices, as_of);
const absent = try common.findAbsentAccounts(allocator, merged, account_map, "schwab", schwab_present.items, prices, as_of);
defer allocator.free(absent);
try common.displayAbsentAccounts(absent, color, "any export", out);
}
if (wells_fargo_present.items.len > 0) {
const absent = try common.findAbsentAccounts(allocator, portfolio, account_map, wf_parser.institution, wells_fargo_present.items, prices, as_of);
const absent = try common.findAbsentAccounts(allocator, merged, account_map, wf_parser.institution, wells_fargo_present.items, prices, as_of);
defer allocator.free(absent);
try common.displayAbsentAccounts(absent, color, "any export", out);
}
@ -1313,11 +1326,11 @@ pub fn runHygieneCheck(
// filter's built-in default applies.
//
// Reuses Section 5's prices; the pipeline loads only the symbols that
// covers nothing for (lots sold since HEAD, sibling portfolio files).
// covers nothing for (lots held at HEAD but sold since).
// Flush first, as before Section 5's load: that top-up, and the
// snapshot-anchor note, go to stderr.
try out.flush();
const known: ?contributions.KnownPrices = if (recon_load) |*r| .{ .requested = recon_syms, .closes = &r.prices } else null;
const known: ?contributions.KnownPrices = if (recon_load) |*r| .{ .requested = live.syms, .closes = &r.prices } else null;
if (try contributions.findUnmatchedLargeLots(io, allocator, env, svc, portfolio_paths, &account_map, as_of, color, refresh, known)) |found| {
var found_mut = found;
defer found_mut.deinit();
@ -1338,9 +1351,8 @@ pub fn runHygieneCheck(
// `splits_current_through` on its metadata.srf row, or its shares
// (and every value derived from them) are misstated across the
// split. Cache-only detection; silent when everything is handled.
if (portfolio.stockSymbols(allocator)) |split_syms| {
defer allocator.free(split_syms);
const unhandled = cli.findUnhandledSplits(svc, allocator, portfolio.lots, split_syms, portfolio_path, as_of, cli.fetchOptionsFromPolicy(refresh));
{
const unhandled = cli.findUnhandledSplits(svc, allocator, merged.lots, live.syms, portfolio_path, as_of, cli.fetchOptionsFromPolicy(refresh));
defer allocator.free(unhandled);
if (unhandled.len > 0) {
try out.print("\n", .{});
@ -1350,7 +1362,7 @@ pub fn runHygieneCheck(
try cli.printFg(out, color, cli.CLR_MUTED, " Add 'splits_current_through::YYYY-MM-DD' (your reconcile date) to {s}'s metadata.srf row.\n", .{nudge.symbol});
}
}
} else |_| {}
}
// ── Section 8: Config file problems ──
//
@ -2328,6 +2340,26 @@ test "findPriceDateMismatches: duplicate HEAD identity collapses to one entry" {
try std.testing.expectApproxEqAbs(@as(f64, 160), mismatches.items[0].new_price, 0.01);
}
/// Test entry to `runHygieneCheck`: the merged load `cli.loadPortfolio`
/// would hand it, minus the `RunCtx` (which only adds split adjustment
/// - no fixture opts in - and printing of skipped records). Defaults
/// `--stale-days 3`, no color, `--refresh-data=never`.
fn runHygieneForTest(
allocator: std.mem.Allocator,
env: *const std.process.Environ.Map,
svc: *zfin.DataService,
paths: []const []const u8,
verbose: bool,
as_of: Date,
now_s: i64,
out: *std.Io.Writer,
) !void {
const io = std.testing.io;
var live = portfolio_loader.loadPortfolioFromPaths(io, allocator, paths, as_of) orelse return error.PortfolioLoadFailed;
defer live.deinit(allocator);
try runHygieneCheck(io, allocator, env, svc, &live, 3, verbose, as_of, now_s, false, .never, out);
}
test "runHygieneCheck: Section 7 flags an un-opted-in symbol's split, not an opted-in one" {
const allocator = std.testing.allocator;
const io = std.testing.io;
@ -2377,7 +2409,7 @@ test "runHygieneCheck: Section 7 flags an un-opted-in symbol's split, not an opt
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, &aw.writer);
const output = aw.written();
try std.testing.expect(std.mem.indexOf(u8, output, "Portfolio hygiene") != null);
@ -2444,7 +2476,7 @@ test "runHygieneCheck: a large deposit on a cash_is_contribution account doesn't
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 5, 11), 1_778_500_000, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 5, 11), 1_778_500_000, &aw.writer);
const out = aw.written();
const start = std.mem.indexOf(u8, out, "Large new lots") orelse return error.LargeLotSectionMissing;
@ -2573,7 +2605,7 @@ fn runSection8(
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, &aw.writer);
return allocator.dupe(u8, aw.written());
}
@ -2807,7 +2839,7 @@ test "runHygieneCheck: discovered exports set unmapped accounts aside unless one
// wall-clock required: discovery keeps only exports modified in the
// last 24 hours, judged against the real mtimes of the files above.
const now_s = std.Io.Timestamp.now(io, .real).toSeconds();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 10, 6), now_s, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 10, 6), now_s, &aw.writer);
const out = aw.written();
// all.csv: the mapped account reconciles clean; the other is named
@ -2864,6 +2896,62 @@ fn writeTwoAccountFixture(io: std.Io, dir: std.Io.Dir, comptime vti_extra: []con
});
}
test "runHygieneCheck: a holding kept in a sibling portfolio file reconciles" {
// The flagless audit used to compare the brokerage to portfolio.srf
// alone, so a holding in portfolio_other.srf read as brokerage-only
// here while `zfin audit --schwab FILE` (which loads the whole glob)
// reconciled it. Both now see the same lots.
const allocator = std.testing.allocator;
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try writeTwoAccountFixture(io, tmp.dir, "");
// Split the fixture's two lots across two files.
try tmp.dir.writeFile(io, .{
.sub_path = "portfolio.srf",
.data = "#!srfv1\nsymbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:200,price:num:300,account::Sample IRA\n",
});
try tmp.dir.writeFile(io, .{
.sub_path = "portfolio_other.srf",
.data = "#!srfv1\nsymbol::SCHD,shares:num:200,open_date::2024-01-15,open_price:num:25,price:num:30,account::Sample Roth\n",
});
var path_buf: [std.fs.max_path_bytes]u8 = undefined;
const dir_len = try tmp.dir.realPathFile(io, ".", &path_buf);
const dir = path_buf[0..dir_len];
var svc = zfin.DataService.init(io, allocator, .{ .cache_dir = dir });
defer svc.deinit();
var env = try std.testing.environ.createMap(allocator);
defer env.deinit();
const pf_path = try std.fs.path.join(allocator, &.{ dir, "portfolio.srf" });
defer allocator.free(pf_path);
const other_path = try std.fs.path.join(allocator, &.{ dir, "portfolio_other.srf" });
defer allocator.free(other_path);
// wall-clock required: discovery keeps only exports modified in the
// last 24 hours, judged against the real mtimes of the files above.
const now_s = std.Io.Timestamp.now(io, .real).toSeconds();
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
// Lexicographic, as the glob resolves: portfolio.srf is the anchor.
try runHygieneForTest(allocator, &env, &svc, &.{ pf_path, other_path }, false, zfin.Date.fromYmd(2026, 10, 6), now_s, &aw.writer);
const out = aw.written();
// SCHD matched the sibling file's lot: a share count to fix, not a
// holding the portfolio is missing.
try std.testing.expect(std.mem.indexOf(u8, out, "Shares +10.000") != null);
try std.testing.expect(std.mem.indexOf(u8, out, "Brokerage only") == null);
try std.testing.expect(std.mem.indexOf(u8, out, " Total: portfolio $36,000.00 brokerage $36,300.00") != null);
// Stale manual prices cover the sibling file too (neither lot has a
// price_date).
const stale = out[(std.mem.indexOf(u8, out, "Stale manual prices") orelse return error.StaleSectionMissing)..];
const stale_end = std.mem.indexOf(u8, stale, "\n\n") orelse stale.len;
try std.testing.expect(std.mem.indexOf(u8, stale[0..stale_end], "SCHD") != null);
try std.testing.expect(std.mem.indexOf(u8, stale[0..stale_end], "VTI") != null);
}
test "runHygieneCheck: reconciliation prices come from the shared load" {
// The other fixtures price every lot by hand, which `stockSymbols`
// excludes, so Section 5 never actually loads. Here VTI has no manual
@ -2904,7 +2992,7 @@ test "runHygieneCheck: reconciliation prices come from the shared load" {
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 10, 6), now_s, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 10, 6), now_s, &aw.writer);
const out = aw.written();
try std.testing.expect(std.mem.indexOf(u8, out, "\n 1 more account, no discrepancies: Sample IRA\n") != null);
@ -2940,7 +3028,7 @@ test "runHygieneCheck: one bad account in an all-accounts export tables only tha
{
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 10, 6), now_s, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 10, 6), now_s, &aw.writer);
const out = aw.written();
try std.testing.expect(std.mem.indexOf(u8, out, " Sample Roth (Sample_Roth, #5678)\n") != null);
@ -2955,7 +3043,7 @@ test "runHygieneCheck: one bad account in an all-accounts export tables only tha
{
var aw: std.Io.Writer.Allocating = .init(allocator);
defer aw.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, true, zfin.Date.fromYmd(2026, 10, 6), now_s, false, .never, &aw.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, true, zfin.Date.fromYmd(2026, 10, 6), now_s, &aw.writer);
const out = aw.written();
try std.testing.expect(std.mem.indexOf(u8, out, " Sample Roth (Sample_Roth, #5678)\n") != null);
@ -3043,7 +3131,7 @@ test "runHygieneCheck: stdout is flushed before each step that can write to stde
var buf: [256]u8 = undefined;
var log = FlushLog.init(allocator, &buf);
defer log.deinit();
try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 10, 6), now_s, false, .never, &log.writer);
try runHygieneForTest(allocator, &env, &svc, &.{pf_path}, false, zfin.Date.fromYmd(2026, 10, 6), now_s, &log.writer);
try log.writer.flush(); // what main does at exit
const out = log.written.items;