lobo/zfin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

make sure large new lots section remains without open_date

Browse source
This commit is contained in:
Emil Lerch 2026-09-24 08:54:47 -07:00
parent 60a42dd4ba
commit b3239eefda
Signed by: lobo
GPG key ID: A7B62D657EF764F8
2 changed files with 134 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
src/commands/audit/hygiene.zig
View file

@ -1244,7 +1244,7 @@ pub fn runHygieneCheck(
// (not in a git repo). Threshold is per-account: an account's
// `audit_large_lot_threshold` in accounts.srf wins, otherwise the
// filter's built-in default applies.
if (contributions.findUnmatchedLargeLots(io, allocator, env, svc, portfolio_paths, &account_map, as_of, color, refresh)) |found| {
if (try contributions.findUnmatchedLargeLots(io, allocator, env, svc, portfolio_paths, &account_map, as_of, color, refresh)) |found| {
var found_mut = found;
defer found_mut.deinit();
@ -2318,6 +2318,68 @@ test "runHygieneCheck: Section 7 flags an un-opted-in symbol's split, not an opt
try std.testing.expect(std.mem.indexOf(u8, sec6, "AMZN") == null);
}
test "runHygieneCheck: a large deposit on a cash_is_contribution account doesn't blank Large new lots" {
// End-to-end regression for the `MissingOpenDate` bug: the deposit
// becomes a `cash_contribution` with no open_date, which used to
// error out of `collectUnmatchedLargeLots` and suppress the whole
// section - taking the unrelated VTI lot down with it.
const allocator = std.testing.allocator;
const io = std.testing.io;
if (!test_git.available(allocator)) return;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
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];
// Committed state: $10k cash on an account whose cash deposits are
// contributions.
try tmp.dir.writeFile(io, .{
.sub_path = "portfolio.srf",
.data = "#!srfv1\nsecurity_type::cash,shares:num:10000,open_date::2025-01-01,open_price:num:1,account::Sample IRA\n",
});
try tmp.dir.writeFile(io, .{
.sub_path = "accounts.srf",
.data = "#!srfv1\naccount::Sample IRA,tax_type::traditional,cash_is_contribution:bool:true\n",
});
try test_git.run(allocator, dir, null, &.{ "init", "-q" });
try test_git.run(allocator, dir, null, &.{ "config", "user.email", "test@example.com" });
try test_git.run(allocator, dir, null, &.{ "config", "user.name", "Test" });
try test_git.run(allocator, dir, null, &.{ "config", "commit.gpgsign", "false" });
try test_git.run(allocator, dir, null, &.{ "add", "portfolio.srf", "accounts.srf" });
try test_git.run(allocator, dir, "2026-01-10T12:00:00", &.{ "commit", "-q", "-m", "A" });
// Working copy: a $50k deposit, plus a $25k new stock lot.
try tmp.dir.writeFile(io, .{
.sub_path = "portfolio.srf",
.data =
\\#!srfv1
\\security_type::cash,shares:num:60000,open_date::2025-01-01,open_price:num:1,account::Sample IRA
\\symbol::VTI,shares:num:100,open_date::2026-05-10,open_price:num:250,account::Sample IRA
\\
,
});
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);
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);
const out = aw.written();
const start = std.mem.indexOf(u8, out, "Large new lots") orelse return error.LargeLotSectionMissing;
const sec = out[start..];
try std.testing.expect(std.mem.indexOf(u8, sec, "VTI") != null);
try std.testing.expect(std.mem.indexOf(u8, sec, "+$50,000.00 (date unknown)") != null);
try std.testing.expect(std.mem.indexOf(u8, sec, "transfer::<DATE>,type::cash,amount:num:50000.00") != null);
}
/// Format a two-account portfolio.srf for the git-history test. Only
/// `shares` varies between revisions, which is enough for
/// findModifiedAccounts to flag the account.

82
src/commands/contributions.zig
View file

@ -311,7 +311,7 @@ pub const meta: framework.Meta = .{
\\
,
.uppercase_first_arg = false,
.user_errors = error{ DuplicateEndpoint, InvalidArg, MissingOpenDate, PrepareFailed, ResolveFailed, UnexpectedArg },
.user_errors = error{ DuplicateEndpoint, InvalidArg, PrepareFailed, ResolveFailed, UnexpectedArg },
};
pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedArgs {
@ -1199,7 +1199,10 @@ pub const UnmatchedLargeLotSet = struct {
/// Mirrors the `zfin contributions` zero-flag path - uses
/// `prepareReport`'s shared git + portfolio + transfer plumbing so
/// the classification is identical. Returns null if the pipeline
/// can't resolve a window (not in a git repo, etc.).
/// can't resolve a window (not in a git repo, etc.) - the expected
/// reason for audit to skip the section, logged at debug level. Any
/// other failure is returned, never folded into that null: doing so
/// once made the whole section disappear over one undated lot.
///
/// The threshold is resolved per lot: an account's
/// `audit_large_lot_threshold` (from `account_map`) wins, otherwise
@ -1221,7 +1224,7 @@ pub fn findUnmatchedLargeLots(
as_of: Date,
color: bool,
refresh: framework.RefreshPolicy,
) ?UnmatchedLargeLotSet {
) !?UnmatchedLargeLotSet {
var arena_state = std.heap.ArenaAllocator.init(allocator);
errdefer arena_state.deinit();
const arena = arena_state.allocator();
@ -1234,16 +1237,14 @@ pub fn findUnmatchedLargeLots(
//
// Separate allocator here so we can tear the whole thing down
// via `arena_state.deinit` once we've copied out the descriptors.
var ctx = prepareReport(io, allocator, arena, env, svc, paths, null, null, as_of, color, refresh, .silent) catch {
var ctx = prepareReport(io, allocator, arena, env, svc, paths, null, null, as_of, color, refresh, .silent) catch |err| {
std.log.scoped(.contributions).debug("large-lot check skipped: no comparable window ({t})", .{err});
arena_state.deinit();
return null;
};
defer ctx.deinit();
const lots = collectUnmatchedLargeLots(arena, ctx.report.changes, account_map) catch {
arena_state.deinit();
return null;
};
const lots = try collectUnmatchedLargeLots(arena, ctx.report.changes, account_map);
return .{ .lots = lots, .arena = arena_state };
}
@ -1297,9 +1298,18 @@ fn collectUnmatchedLargeLots(
const residual = c.attributedValue();
if (residual < threshold) continue;
// open_date is populated in Pass 1's new-lot branch; absence
// here would be a pipeline bug.
const od = c.open_date orelse return error.MissingOpenDate;
// Pass 1's new-lot branch sets open_date. A `cash_contribution`
// never has one: it is a balance INCREASE on an existing cash
// lot, and that lot's open_date is when the account was set
// up, not when this money arrived. `Date.epoch` is the
// codebase's "no date" (import writes it; `Lot.fromParsed`
// fills it), and `printLargeLotWarning` renders it as "date
// unknown" with a `<DATE>` placeholder rather than a real date.
//
// This used to `return error.MissingOpenDate`, which the caller
// collapsed into "no section" - one large deposit to a
// `cash_is_contribution` account blanked the whole list.
const od = c.open_date orelse Date.epoch;
const account_copy = try arena.dupe(u8, c.account);
const symbol_copy = try arena.dupe(u8, c.symbol);
try out.append(arena, .{
@ -8272,3 +8282,53 @@ test "printReport: full report renders every section and sub-printer" {
try printReport(&cw, &report, "portfolio.srf", true);
try testing.expect(std.mem.indexOf(u8, cw.buffered(), "\x1b[") != null);
}
test "collectUnmatchedLargeLots: a large cash_contribution surfaces, undated, alongside the rest" {
// Regression. A positive balance change on a `cash_is_contribution`
// account is reclassified to `cash_contribution`, which carries no
// open_date. That returned error.MissingOpenDate, the caller turned
// it into "no section", and audit's entire "Large new lots" list -
// including the unrelated stock lot below - silently vanished.
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();
var am = try analysis.parseAccountsFile(
allocator,
"#!srfv1\naccount::Sample IRA,tax_type::traditional,cash_is_contribution:bool:true\n",
);
const before = [_]Lot{
.{ .symbol = "cash", .shares = 10_000, .open_date = Date.fromYmd(2025, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
};
const after = [_]Lot{
.{ .symbol = "cash", .shares = 60_000, .open_date = Date.fromYmd(2025, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
.{ .symbol = "VTI", .shares = 100, .open_date = Date.fromYmd(2026, 5, 10), .open_price = 250.0, .account = "Sample IRA" },
};
const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 11), .{ .account_map = &am });
// Precondition: the balance change really was reclassified.
var saw_contribution = false;
for (report.changes) |c| {
if (c.kind == .cash_contribution) saw_contribution = true;
}
try std.testing.expect(saw_contribution);
const lots = try collectUnmatchedLargeLots(allocator, report.changes, null);
try std.testing.expectEqual(@as(usize, 2), lots.len);
var cash_seen = false;
var stock_seen = false;
for (lots) |l| {
switch (l.security_type) {
.cash => {
cash_seen = true;
try std.testing.expectApproxEqAbs(@as(f64, 50_000.0), l.value, 0.01);
try std.testing.expect(l.open_date.eql(Date.epoch));
},
.stock => stock_seen = true,
else => {},
}
}
try std.testing.expect(cash_seen and stock_seen);
}