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

fix regression in contribution

Browse source
This commit is contained in:
Emil Lerch 2026-08-18 01:38:57 -07:00
parent c986df83c1
commit 5706eed3c1
Signed by: lobo
GPG key ID: A7B62D657EF764F8
1 changed files with 249 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

269
src/commands/contributions.zig
View file

@ -2508,30 +2508,50 @@ fn matchCashDestination(
});
}
/// Best-effort: find a negative cash_delta or lot_removed on the
/// `from` account with |value| >= amount - tolerance; reclassify to
/// transfer_out. No-op (silent) if no such Change exists - the
/// sending side may not be in portfolio.srf.
/// Best-effort: find the cash that left the `from` account and
/// reclassify it to `transfer_out`. No-op (silent) if no such Change
/// exists - the sending side may not be in portfolio.srf at all, which
/// the format explicitly allows.
///
/// Restricted to CASH outflows: a negative `cash_delta`, or a removed /
/// closed cash lot (a line drained in full). A `type::cash` record says
/// dollars moved, so a security sale is never its sending leg - selling
/// to raise the cash is a separate event, and the cash decrease is the
/// transfer. Securities moving between accounts is what `type::in_kind`
/// and `matchInKindTransfer` are for, matched by symbol on both sides.
///
/// The type gate is load-bearing rather than tidy-up. This scan was
/// unreachable while `value()` was 0 for every removal; once outflows
/// carried dollars it went live, and an unrelated sale in the same
/// account became a candidate. Consuming it here would flip it out of
/// `lot_removed`, drop it from the sale-proceeds budget in
/// `matchIntraAccountPurchases`, and make the repurchase it funded read
/// as new money - reintroducing the bug that budget exists to fix.
fn tryMatchFromSide(
changes: *std.ArrayList(Change),
rec: transaction_log.TransferRecord,
) void {
for (changes.items) |*c| switch (c.kind) {
.cash_delta, .lot_removed, .position_closed => {
if (!std.mem.eql(u8, c.account, rec.from)) continue;
const abs_val = @abs(c.value());
if (abs_val < rec.amount - transfer_amount_tolerance) continue;
// Match: flip to transfer_out. We don't track
// transfer_attributed on the from side (it's
// decorative) but we record the counterpart date so
// the Transfers section can cross-reference.
c.kind = .transfer_out;
c.transfer_attributed = rec.amount;
c.transfer_date = rec.transfer;
return;
},
else => {},
};
for (changes.items) |*c| {
if (c.security_type != .cash) continue;
if (!std.mem.eql(u8, c.account, rec.from)) continue;
// Must be money leaving. `cash_delta` is signed, so an account
// that GAINED cash must not be read as a sending leg; removals
// and closes are outflows by construction.
const outflow: f64 = switch (c.kind) {
.cash_delta => if (c.value() < 0) -c.value() else continue,
.lot_removed, .position_closed => c.face_value,
else => continue,
};
if (outflow < rec.amount - transfer_amount_tolerance) continue;
// Match: flip to transfer_out. We don't track
// transfer_attributed on the from side (it's decorative) but we
// record the counterpart date so the Transfers section can
// cross-reference.
c.kind = .transfer_out;
c.transfer_attributed = rec.amount;
c.transfer_date = rec.transfer;
return;
}
}
/// Match a `type::in_kind` transfer record: securities moved between
@ -4291,6 +4311,19 @@ test "computeReport: transfer record takes priority over intra-account netting"
}
}
/// Sum of everything a window reports as new money. Mirrors
/// `summarizeAttribution`'s buckets without needing a ReportContext, so
/// classifier-level tests can assert on the same figure the CLI prints.
fn attributionTotalForTest(report: Report) f64 {
var total: f64 = 0;
for (report.changes) |c| switch (c.kind) {
.new_stock, .new_cash, .new_cd, .new_option, .cash_contribution, .partial_transfer_in => total += c.attributedValue(),
.new_drip_lot, .drip_confirmed, .rollup_delta => total += c.value(),
else => {},
};
return total;
}
// ── Security-sale funding (intra-account reallocation) ───────
//
// Case labels A-F below match the scenario table worked out when this
@ -4867,6 +4900,202 @@ test "printCollapsedSales: many retired lots render as one line" {
try std.testing.expect(std.mem.indexOf(u8, text, "$4,800.00") != null);
}
test "computeReport: a declared cash transfer out does not consume a security sale" {
// `tryMatchFromSide` looks for the sending leg of a declared
// `type::cash` transfer. Its removal arm was written for a fully
// drained CASH lot, whose dollars live in `face_value` - but it was
// unreachable while `value()` was 0 for every removal, so nothing
// pinned that intent.
//
// Now that outflows carry dollars the arm is live, and an unrelated
// security sale in the same account is a candidate. If it wins, the
// sale flips to `transfer_out`, drops out of the sale-proceeds
// budget, and the rebuy it paid for reads as new money - the exact
// bug this whole change set exists to remove.
//
// Here: $50k of cash is declared as moving out of Sample IRA, and
// separately SYM is sold for $100k and NEWSYM bought with the
// proceeds. The transfer must take the cash; the reallocation must
// still net to zero.
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("SYM", 100.0);
const before = [_]Lot{
.{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample IRA" },
.{ .symbol = "", .shares = 50_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
};
const after = [_]Lot{
// SYM sold, proceeds straight into NEWSYM.
.{ .symbol = "NEWSYM", .shares = 2_000, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 50.0, .account = "Sample IRA" },
// The $50k cash left, as declared.
.{ .symbol = "", .shares = 0, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
.{ .symbol = "", .shares = 50_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Roth IRA" },
};
const tlog = try transaction_log.parseTransactionLogFile(allocator,
\\#!srfv1
\\transfer::2026-05-02,type::cash,amount:num:50000,from::Sample IRA,to::Sample Roth IRA,dest_lot::cash
\\
);
const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{
.transfer_log = tlog.transfers,
});
// The SYM sale must remain a sale, not be eaten as the transfer's
// sending leg.
var sale: ?Change = null;
var buy: ?Change = null;
for (report.changes) |c| {
if (std.mem.eql(u8, c.symbol, "SYM") and isSaleKind(c)) sale = c;
if (c.kind == .new_stock and std.mem.eql(u8, c.symbol, "NEWSYM")) buy = c;
}
try std.testing.expect(sale != null);
try std.testing.expectApproxEqAbs(@as(f64, 100_000.0), sale.?.face_value, 0.01);
// ...so its proceeds still fund the rebuy, which is not new money.
try std.testing.expect(buy != null);
try std.testing.expectApproxEqAbs(@as(f64, 100_000.0), buy.?.internal_funded, 0.01);
try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.attributedValue(), 0.01);
}
test "computeReport: an untracked transfer source does not consume a security sale" {
// The dangerous shape. A `type::cash` transfer's sending account is
// allowed to be unmodelled - the docs say a missing source side is
// not an error. But `tryMatchFromSide` scans for ANY qualifying
// outflow on that account, so with no cash change to claim, an
// unrelated security sale is the only candidate left.
//
// Flipping it to `transfer_out` removes it from the sale-proceeds
// budget, and the rebuy it paid for reads as new money again. The
// sending leg of a CASH transfer must be cash: a fully drained cash
// lot (whose dollars live in `face_value`) or a negative
// `cash_delta`, never a security.
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("SYM", 100.0);
const before = [_]Lot{
.{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample IRA" },
};
const after = [_]Lot{
// Reallocation inside Sample IRA: SYM out, NEWSYM in.
.{ .symbol = "NEWSYM", .shares = 2_000, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 50.0, .account = "Sample IRA" },
// The declared transfer's destination. Its source cash is not
// modelled in the portfolio at all.
.{ .symbol = "", .shares = 50_000, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Sample Roth IRA" },
};
const tlog = try transaction_log.parseTransactionLogFile(allocator,
\\#!srfv1
\\transfer::2026-05-02,type::cash,amount:num:50000,from::Sample IRA,to::Sample Roth IRA,dest_lot::cash
\\
);
const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{
.transfer_log = tlog.transfers,
});
var sale: ?Change = null;
var buy: ?Change = null;
for (report.changes) |c| {
if (std.mem.eql(u8, c.symbol, "SYM")) sale = c;
if (c.kind == .new_stock and std.mem.eql(u8, c.symbol, "NEWSYM")) buy = c;
}
// The sale stays a sale.
try std.testing.expect(sale != null);
try std.testing.expectEqual(ChangeKind.lot_removed, sale.?.kind);
// And still funds the rebuy, so no new money is reported.
try std.testing.expect(buy != null);
try std.testing.expectApproxEqAbs(@as(f64, 100_000.0), buy.?.internal_funded, 0.01);
try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.attributedValue(), 0.01);
}
test "computeReport: a fully drained cash lot is the transfer's sending leg" {
// The shape `tryMatchFromSide`'s removal arm was written for, and
// which never worked: when the cash line is spent to the cent the
// user deletes it, so the outflow arrives as a removed cash lot
// rather than a negative `cash_delta`. Its dollars live in
// `face_value` because `value()` was 0 for removals - which is
// exactly why the arm could never fire.
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 = "", .shares = 50_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
};
const after = [_]Lot{
.{ .symbol = "", .shares = 50_000, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Sample Roth IRA" },
};
const tlog = try transaction_log.parseTransactionLogFile(allocator,
\\#!srfv1
\\transfer::2026-05-02,type::cash,amount:num:50000,from::Sample IRA,to::Sample Roth IRA,dest_lot::cash
\\
);
const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{
.transfer_log = tlog.transfers,
});
var n_out: usize = 0;
for (report.changes) |c| {
if (c.kind == .transfer_out and std.mem.eql(u8, c.account, "Sample IRA")) n_out += 1;
}
try std.testing.expectEqual(@as(usize, 1), n_out);
// And the whole move contributes nothing.
try std.testing.expectApproxEqAbs(@as(f64, 0.0), attributionTotalForTest(report), 0.01);
}
test "computeReport: an account that GAINED cash is not a transfer's sending leg" {
// `cash_delta` is signed and the match used to take its magnitude,
// so an account whose cash went UP could be reported as the source
// of a transfer out. Money leaving is the whole point, so the sign
// has to be checked.
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 = "", .shares = 10_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
};
const after = [_]Lot{
// Sample IRA's cash rose by $60k - it cannot be what funded a
// transfer out of it.
.{ .symbol = "", .shares = 70_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample IRA" },
.{ .symbol = "", .shares = 50_000, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Sample Roth IRA" },
};
const tlog = try transaction_log.parseTransactionLogFile(allocator,
\\#!srfv1
\\transfer::2026-05-02,type::cash,amount:num:50000,from::Sample IRA,to::Sample Roth IRA,dest_lot::cash
\\
);
const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{
.transfer_log = tlog.transfers,
});
for (report.changes) |c| {
try std.testing.expect(c.kind != .transfer_out);
}
}
test "prepareReport: a lot archived into a sibling portfolio file is one sale" {
// End-to-end pin on the multi-file diff. The documented workflow for
// a sale is to move the closed lot out of `portfolio.srf` into a