From 3dd72c2e3ec69d397ef6b48009d7b7851021711a Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sat, 8 Aug 2026 17:09:16 -0700 Subject: [PATCH] match transfers if moved cash->cash and stock purchased in same period --- src/commands/contributions.zig | 203 ++++++++++++++++++++++++++++----- 1 file changed, 174 insertions(+), 29 deletions(-) diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index e5054ca..73d71bd 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -2088,6 +2088,13 @@ fn matchTransfers( // order. Underflow past tolerance -> unmatched_transfer. var cash_budget: std.StringHashMap(f64) = .init(allocator); defer cash_budget.deinit(); + + // Shortfalls carried out of the cash-destination path, per destination + // account, drawn down against new lots once every record has been seen. + // Deferred to the end so a second transfer into the same account can add to + // the same budget before any of it is spent. + var transfer_funding: std.StringHashMap(FundingShortfall) = .init(allocator); + defer transfer_funding.deinit(); for (changes.items) |c| { const v = c.value(); switch (c.kind) { @@ -2125,12 +2132,16 @@ fn matchTransfers( try matchLotDestination(allocator, changes, &consumed_lot_idx, rec, dl); }, .cash => { - try matchCashDestination(allocator, changes, &cash_budget, cash_attributed_by_account, rec); + try matchCashDestination(allocator, changes, &cash_budget, cash_attributed_by_account, &transfer_funding, rec); }, } tryMatchFromSide(changes, rec); } + + // After every record, so multiple transfers into one account pool their + // funding before it is drawn against that account's purchases. + try matchTransferFundedPurchases(allocator, changes, &transfer_funding); } /// Append a synthetic `unmatched_transfer` Change carrying the record's @@ -2292,31 +2303,44 @@ fn matchCashDestination( changes: *std.ArrayList(Change), cash_budget: *std.StringHashMap(f64), cash_attributed_by_account: *std.StringHashMap(f64), + transfer_funding: *std.StringHashMap(FundingShortfall), rec: transaction_log.TransferRecord, ) !void { const budget_entry = cash_budget.getPtr(rec.to); - const available = if (budget_entry) |p| p.* else 0.0; - if (available < rec.amount - transfer_amount_tolerance) { - const buf = try std.fmt.allocPrint( - allocator, - "destination cash increase ${d:.2} insufficient for transfer ${d:.2}", - .{ available, rec.amount }, - ); - try appendUnmatchedWithOwnedNote(allocator, changes, rec, buf); - return; + const available = @max(0.0, if (budget_entry) |p| p.* else 0.0); + + // Credit whatever cash actually showed up, and carry the rest as a funding + // budget for the destination's new lots. + // + // This used to bail out entirely when the cash increase fell short, which + // meant a transfer whose cash was invested inside the same window credited + // nothing at all and every purchase it funded read as new money. The cash + // is genuinely absent from the snapshot in that case - it arrived and left + // between two commits - so the shortfall is expected, not a discrepancy. + // See `matchTransferFundedPurchases`, which draws it down and flags only + // what the account's new lots cannot absorb. + const credited = @min(available, rec.amount); + const shortfall = rec.amount - credited; + if (shortfall > transfer_amount_tolerance) { + const gop = try transfer_funding.getOrPut(rec.to); + if (!gop.found_existing) gop.value_ptr.* = .{ .rec = rec }; + gop.value_ptr.*.amount += shortfall; + gop.value_ptr.*.declared += rec.amount; } // Draw from the budget. Running remainder stays on the budget // so later records on the same account see the correct // capacity. - if (budget_entry) |p| p.* -= rec.amount; + if (budget_entry) |p| p.* -= credited; // Accumulate into per-account attribution bucket. The per- // account totals pass subtracts this from cash-side totals so // transferred cash doesn't double-count. const gop = try cash_attributed_by_account.getOrPut(rec.to); if (!gop.found_existing) gop.value_ptr.* = 0; - gop.value_ptr.* += rec.amount; + // Only the cash that was actually observed; the rest is attributed to the + // new lots instead, so adding the full amount here would double-count. + gop.value_ptr.* += credited; // Distribute the record amount across the destination account's // cash-side Changes by accumulating into each Change's @@ -2335,7 +2359,7 @@ fn matchCashDestination( // pass continues to use `cash_attributed_by_account` for its // per-account math - the two views agree because the same // amount is subtracted on both sides. - var remaining = rec.amount; + var remaining = credited; for (changes.items) |*c| { if (remaining <= 0) break; if (!std.mem.eql(u8, c.account, rec.to)) continue; @@ -2576,6 +2600,79 @@ fn matchInKindTransfer( /// several purchase lots; the total netted is the same regardless of /// order, but which specific lot shows a residual can vary. This /// mirrors `matchCashDestination`'s order-dependent draw. +/// Draw `budget` down against the new purchase lots in `account`, marking the +/// funded portion on each. Returns whatever the account's lots could not +/// absorb. +/// +/// Shared by the two things that can fund a purchase without it being new +/// money: cash that visibly left the same account, and a declared transfer +/// whose cash was spent before it could be observed. The drawdown is identical; +/// only the meaning of a leftover differs, which is why the callers handle the +/// return value differently rather than this function deciding. +fn drawDownAgainstNewLots(changes: *std.ArrayList(Change), account: []const u8, budget: f64) f64 { + var remaining = budget; + for (changes.items) |*c| { + if (remaining <= 0) break; + switch (c.kind) { + .new_stock, .new_cd => {}, + else => continue, + } + if (!std.mem.eql(u8, c.account, account)) continue; + const unattributed = c.attributedValue(); + if (unattributed <= 0) continue; + const draw = @min(unattributed, remaining); + c.internal_funded += draw; + remaining -= draw; + } + return remaining; +} + +/// Attribute purchases funded by a declared transfer whose cash never appeared +/// in a snapshot. +/// +/// A `transfer` record says money moved from A to B. `matchCashDestination` +/// credits it against an observed cash increase in B - but when the cash is +/// invested inside the same reconcile window, no snapshot ever contains it: the +/// diff sees new security lots in B and a few dollars of leftover cash. The +/// transfer then failed its cash check and the purchases counted as fresh +/// money, which is how one 401(k)-to-BrokerageLink move reported $738,814 of +/// contributions that were nothing of the kind. +/// +/// So the shortfall becomes a funding budget for that account's new lots - +/// exactly what `matchIntraAccountPurchases` does with an observed cash +/// decrease, seeded from the operator's declaration instead of from an +/// observation. Anything the lots cannot absorb is still flagged: a transfer +/// claiming more than the destination gained is a real discrepancy and must not +/// be silently swallowed. +fn matchTransferFundedPurchases( + allocator: std.mem.Allocator, + changes: *std.ArrayList(Change), + funding: *std.StringHashMap(FundingShortfall), +) !void { + var it = funding.iterator(); + while (it.next()) |entry| { + const account = entry.key_ptr.*; + const sf = entry.value_ptr.*; + if (sf.amount <= transfer_amount_tolerance) continue; + const leftover = drawDownAgainstNewLots(changes, account, sf.amount); + if (leftover <= transfer_amount_tolerance) continue; + const buf = try std.fmt.allocPrint( + allocator, + "transfer of ${d:.2} exceeds the destination's cash increase and new lots by ${d:.2}", + .{ sf.declared, leftover }, + ); + try appendUnmatchedWithOwnedNote(allocator, changes, sf.rec, buf); + } +} + +/// Per-account transfer shortfall, plus the record it came from so an +/// unabsorbed remainder can be reported against the right transfer. +const FundingShortfall = struct { + amount: f64 = 0, + declared: f64 = 0, + rec: transaction_log.TransferRecord, +}; + fn matchIntraAccountPurchases( allocator: std.mem.Allocator, changes: *std.ArrayList(Change), @@ -2606,19 +2703,12 @@ fn matchIntraAccountPurchases( } if (outflow.count() == 0) return; - // Draw each account's outflow down against its new purchase lots. - for (changes.items) |*c| { - switch (c.kind) { - .new_stock, .new_cd => {}, - else => continue, - } - const budget = outflow.getPtr(c.account) orelse continue; - if (budget.* <= 0) continue; - const unattributed = c.attributedValue(); // value() minus any prior attribution - if (unattributed <= 0) continue; - const draw = @min(unattributed, budget.*); - c.internal_funded += draw; - budget.* -= draw; + // Draw each account's outflow down against its new purchase lots. A + // leftover here is unremarkable - cash can leave an account for reasons + // other than buying something - so it is simply discarded. + var oit = outflow.iterator(); + while (oit.next()) |e| { + _ = drawDownAgainstNewLots(changes, e.key_ptr.*, e.value_ptr.*); } } @@ -5068,7 +5158,56 @@ test "matchTransfers: amount exceeds lot value emits unmatched" { try std.testing.expectEqual(@as(usize, 1), n_unmatched); } -test "matchTransfers: cash insufficient emits unmatched" { +test "matchTransfers: a transfer spent on securities before any snapshot is not new money" { + 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", 100.0); + + // The real 2026-08 shape, scaled down. $50k moved into an account and was + // invested the same week, so no snapshot ever contains the cash: the diff + // sees a new security lot plus the few dollars that did not get spent. + const before = [_]Lot{}; + const after = [_]Lot{ + .{ .symbol = "VOO", .shares = 499, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 100.0, .account = "Acct B" }, + .{ .symbol = "cash", .shares = 100, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Acct B" }, + }; + + const tlog = try transaction_log.parseTransactionLogFile(allocator, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:50000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ + .transfer_log = tlog.transfers, + }); + + // Nothing new entered the portfolio: $49,900 of VOO plus $100 of leftover + // cash is exactly the $50,000 that moved. Before this was handled, the + // purchase counted as a fresh contribution - the mechanism that reported + // $738,814 of contributions for a 401(k)-to-BrokerageLink move. + const t = report.account_totals.get("Acct B").?; + try std.testing.expectApproxEqAbs(@as(f64, 0.0), t.new_money, 0.01); + + // And it is not reported as a discrepancy either, because it is not one: + // the destination gained precisely what the record declared. + for (report.changes) |c| { + try std.testing.expect(c.kind != .unmatched_transfer); + } + + // The purchase is attributed as internally funded rather than being + // dropped, so it still shows under "Internal purchases". + var funded: f64 = 0; + for (report.changes) |c| { + if (c.kind == .new_stock) funded += c.internal_funded; + } + try std.testing.expectApproxEqAbs(@as(f64, 49900.0), funded, 0.01); +} + +test "matchTransfers: a short cash increase credits what arrived and flags the gap" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const allocator = arena_state.allocator(); @@ -5091,14 +5230,20 @@ test "matchTransfers: cash insufficient emits unmatched" { .transfer_log = tlog.transfers, }); + // The $2k the destination never gained is still a real discrepancy. var n_unmatched: usize = 0; for (report.changes) |c| if (c.kind == .unmatched_transfer) { n_unmatched += 1; }; try std.testing.expectEqual(@as(usize, 1), n_unmatched); - // new_cash stays unchanged; $3k still counts as new_money. + + // But the $3k that DID arrive is transferred money, not new money. + // Previously the whole record was abandoned when the amounts disagreed, + // so a correct partial attribution was discarded and the $3k was reported + // as a fresh contribution - which it demonstrably is not, since a transfer + // record says where it came from. const t = report.account_totals.get("Acct B").?; - try std.testing.expectApproxEqAbs(@as(f64, 3000.0), t.new_money, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), t.new_money, 0.01); } test "matchTransfers: same-day multi-cash records drain a single cash_delta" {