From 04cf12d12e8ed30b7dfd98369b6357b1c26bb30b Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sat, 23 May 2026 11:25:12 -0700 Subject: [PATCH] fix transaction log matcher reflecting real world usage --- src/commands/audit.zig | 48 ++- src/commands/contributions.zig | 723 +++++++++++++++++++++++++-------- src/models/transaction_log.zig | 182 ++++++++- 3 files changed, 762 insertions(+), 191 deletions(-) diff --git a/src/commands/audit.zig b/src/commands/audit.zig index efb82ec..b77c3ac 100644 --- a/src/commands/audit.zig +++ b/src/commands/audit.zig @@ -1269,26 +1269,27 @@ fn printLargeLotWarning( try cli.printFg(out, color, cli.CLR_MUTED, " If this was an external contribution: no action needed.\n", .{}); try cli.printFg(out, color, cli.CLR_MUTED, " If this was an internal transfer, add to transaction_log.srf:\n", .{}); - // Amount formatted as a whole-dollar number for the `num:` - // encoding; precise-to-the-cent values are rare in practice - // and callers can edit the template if needed. - const amount_int: i64 = @intFromFloat(@round(lot.value)); - + // Amount formatted with cents precision so the suggested + // `amount:num:N` exactly matches the lot's value. The matcher + // has a $1 tolerance so a whole-dollar suggestion would usually + // pair, but pasting a value that lies about the actual lot is + // a poor user experience — `transaction_log.srf` should record + // what actually moved. if (lot.security_type == .cash) { try cli.printFg( out, color, cli.CLR_MUTED, - " transfer::{s},type::cash,amount:num:{d},from::,to::{s},dest_lot::cash\n", - .{ date_str, amount_int, lot.account }, + " transfer::{s},type::cash,amount:num:{d:.2},from::,to::{s},dest_lot::cash\n", + .{ date_str, lot.value, lot.account }, ); } else { try cli.printFg( out, color, cli.CLR_MUTED, - " transfer::{s},type::cash,amount:num:{d},from::,to::{s},dest_lot::{s}@{s}\n", - .{ date_str, amount_int, lot.account, lot.symbol, date_str }, + " transfer::{s},type::cash,amount:num:{d:.2},from::,to::{s},dest_lot::{s}@{s}\n", + .{ date_str, lot.value, lot.account, lot.symbol, date_str }, ); } } @@ -2558,7 +2559,7 @@ test "printLargeLotWarning: cash destination emits dest_lot::cash template" { try std.testing.expect(std.mem.indexOf(u8, output, "on 2026-05-10") != null); // Template line with the expected SRF shape. - try std.testing.expect(std.mem.indexOf(u8, output, "transfer::2026-05-10,type::cash,amount:num:50000,from::,to::Acct A,dest_lot::cash") != null); + try std.testing.expect(std.mem.indexOf(u8, output, "transfer::2026-05-10,type::cash,amount:num:50000.00,from::,to::Acct A,dest_lot::cash") != null); } test "printLargeLotWarning: stock destination emits dest_lot::SYM@DATE template" { @@ -2578,7 +2579,32 @@ test "printLargeLotWarning: stock destination emits dest_lot::SYM@DATE template" try std.testing.expect(std.mem.indexOf(u8, output, "Acct B: new STOCK lot SYM") != null); try std.testing.expect(std.mem.indexOf(u8, output, "+$25,000.00") != null); - try std.testing.expect(std.mem.indexOf(u8, output, "transfer::2026-05-03,type::cash,amount:num:25000,from::,to::Acct B,dest_lot::SYM@2026-05-03") != null); + try std.testing.expect(std.mem.indexOf(u8, output, "transfer::2026-05-03,type::cash,amount:num:25000.00,from::,to::Acct B,dest_lot::SYM@2026-05-03") != null); +} + +test "printLargeLotWarning: cents are preserved in template" { + // Regression: previously the template rounded to whole dollars, + // so a $73,158.33 lot suggested `amount:num:73158`. Pasting that + // verbatim into transaction_log.srf records a fictitious amount + // and (with $1 matcher tolerance) only barely pairs. The fix + // prints two-decimal precision so the suggested record exactly + // describes the lot it's offering to attribute. + var buf: [1024]u8 = undefined; + var writer = std.Io.Writer.fixed(&buf); + + const lot: contributions.UnmatchedLargeLot = .{ + .account = "Joint trust", + .symbol = "", + .security_type = .cash, + .value = 73_158.33, + .open_date = Date.fromYmd(2026, 5, 20), + }; + + try printLargeLotWarning(&writer, lot, false); + const output = writer.buffered(); + + try std.testing.expect(std.mem.indexOf(u8, output, "amount:num:73158.33") != null); + try std.testing.expect(std.mem.indexOf(u8, output, "amount:num:73158,") == null); } test "strLessThan: orders strings lexicographically" { diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index d86547a..9dfcdee 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -125,11 +125,10 @@ //! effort: a missing `from` side is //! silent, not an error (the sending //! account may not be in portfolio.srf). -//! - `unmatched_transfer` — record in the window that couldn't -//! be matched. Surfaces in the Flagged -//! section with a reason string; the -//! transfer amount stays out of -//! attribution either way. +//! - `unmatched_transfer` — record that couldn't be matched. +//! Surfaces in the Flagged section with +//! a reason string; the transfer amount +//! stays out of attribution either way. //! //! | Scenario | Kind | Section | In Grand Total | In Attribution | //! |-----------------------------------------------|------------------------|------------------|:--------------:|:--------------:| @@ -146,6 +145,34 @@ //! `Report.cash_attributed_by_account`, which the per-account totals //! and attribution summary subtract from cash-side contributions. //! +//! ### Which records does the matcher consider? +//! +//! The matcher runs against the records that are NEW in the after-side +//! `transaction_log.srf` relative to the before-side. Concretely: +//! `prepareReport` loads the file at both `before_rev` and the +//! after-side (working copy or `after_rev`), parses each, and passes +//! the set difference (`after - before`, by `TransferRecord.eql`) to +//! the matcher. See `diffTransferLogs`. +//! +//! Why not filter by `transfer::DATE` against the diff's git +//! timestamp window? Because the user's natural workflow is to +//! record a transfer days, weeks, or months after the actual +//! transaction date. A back-dated record is the rule, not the +//! exception. The original date-window filter rejected those +//! back-dated records and produced "unmatched contribution" noise +//! the user couldn't quiet without changing the record's date +//! to fall inside the diff window — a workaround that destroyed +//! the historical accuracy of `transaction_log.srf`. +//! +//! Editing a previously-recorded transfer (e.g. fixing a typo in +//! `from::`) produces an old-form record in `before` and a new-form +//! record in `after`. The old form silently drops out (its diff cycle +//! is over); the new form is treated as a fresh record and re-pairs +//! against the current diff. If no matching destination Change exists +//! in the current diff (because the lot was added in an earlier +//! commit), the record surfaces as `unmatched_transfer` — accept the +//! noise or undo the edit. +//! //! The matcher also composes with `direct_indexing::true`: a transfer //! into a direct-indexing account matches normally on the destination //! lot; subsequent tracking-error drift on that lot is still swallowed @@ -486,27 +513,46 @@ fn prepareReport( var account_map_opt = svc.loadAccountMap(portfolio_path); defer if (account_map_opt) |*am| am.deinit(); - // Load transaction_log.srf (if present) so the matchTransfers - // pass can reclassify destination/source Changes. Absent file → - // no-op. Derive window endpoints from the committer-date of the - // before/after revs; fall back to today when the after side is - // working-copy or when the git timestamp lookup fails (failure - // silently degrades to an unbounded-above window). - var transfer_log_opt = svc.loadTransferLog(portfolio_path); - defer if (transfer_log_opt) |*tl| tl.deinit(); - - const window_start: ?Date = blk: { - const ts = git.commitTimestamp(io, arena, repo.root, endpoints.range.before_rev) catch break :blk null; - break :blk Date.fromEpoch(ts); + // Load transaction_log.srf from BOTH sides of the diff. The + // matcher considers only records that are NEW in the after-side + // log (i.e. added to `transaction_log.srf` since `before_rev`). + // This decouples record matching from the record's + // `transfer::DATE`, allowing back-dated entries to pair with + // the diff that introduced them. + // + // Path is sibling to portfolio.srf in the same git repo. + // Missing-on-either-side is OK: file may not have existed in + // before_rev (treat as empty) or may not exist in working copy + // (treat as no records → matcher is a no-op). + const tlog_rel_path = blk: { + const dir_end = if (std.mem.lastIndexOfScalar(u8, repo.rel_path, '/')) |idx| idx + 1 else 0; + break :blk std.fmt.allocPrint(arena, "{s}transaction_log.srf", .{repo.rel_path[0..dir_end]}) catch return error.PrepareFailed; }; - const window_end: ?Date = blk: { + + var before_tlog_opt: ?transaction_log.TransactionLog = blk: { + const data = git.show(io, arena, repo.root, endpoints.range.before_rev, tlog_rel_path) catch break :blk null; + break :blk transaction_log.parseTransactionLogFile(arena, data) catch null; + }; + defer if (before_tlog_opt) |*tl| tl.deinit(); + + var after_tlog_opt: ?transaction_log.TransactionLog = blk: { if (endpoints.range.after_rev) |rev| { - const ts = git.commitTimestamp(io, arena, repo.root, rev) catch break :blk as_of; - break :blk Date.fromEpoch(ts); + const data = git.show(io, arena, repo.root, rev, tlog_rel_path) catch break :blk null; + break :blk transaction_log.parseTransactionLogFile(arena, data) catch null; } else { - break :blk as_of; + break :blk svc.loadTransferLog(portfolio_path); } }; + defer if (after_tlog_opt) |*tl| tl.deinit(); + + // Diff: keep only the records new in after. The slice borrows + // from after_tlog_opt's record memory; both must outlive the + // matcher call below (they do — same arena scope). + const new_records: ?[]const transaction_log.TransferRecord = blk: { + const after_tl = if (after_tlog_opt) |*tl| tl else break :blk null; + const before_ptr: ?*const transaction_log.TransactionLog = if (before_tlog_opt) |*tl| tl else null; + break :blk diffTransferLogs(arena, before_ptr, after_tl) catch return error.PrepareFailed; + }; const report = computeReport( arena, @@ -516,9 +562,7 @@ fn prepareReport( as_of, .{ .account_map = if (account_map_opt) |*am| am else null, - .transfer_log = if (transfer_log_opt) |*tl| tl else null, - .window_start = window_start, - .window_end = window_end, + .transfer_log = new_records, }, ) catch { if (verbosity == .verbose) cli.stderrPrint(io, "Error computing contributions diff.\n"); @@ -932,6 +976,38 @@ pub fn findUnmatchedLargeLots( /// and dupe their string fields into `arena`. Split out so tests can /// feed a synthetic `[]Change` without running the git/IO pipeline. /// +/// Cash-destination transfers don't flip their original `new_cash` / +/// `cash_contribution` Change (a single cash delta can be drained by +/// multiple records, which the kind field can't represent). Instead +/// the matcher records the attributed amount in +/// `cash_attributed_by_account`. Subtract that here so a fully- +/// attributed cash lot doesn't re-surface as "unmatched large lot." +/// A partially-attributed cash lot surfaces only on the residual, +/// matching the user's mental model: "this much of the lot is +/// already documented, check the rest." +/// +/// Deliberately excludes `partial_transfer_in`. A partial lot already +/// has an explicit transfer record acknowledging the large movement; +/// the unmatched residual is typically small (pre-existing cash that +/// topped the lot off) and surfacing it again would nag on something +/// the user has already documented. If a residual is large enough to +/// care about independently, the user can review the lot's full value +/// via `zfin contributions` — this filter's job is to catch +/// *unrecorded* large movements, not to re-flag partial ones. +/// Pure filter: pick out new-side Changes whose unattributed value +/// (`attributedValue()`) is at or above `threshold`, and dupe their +/// string fields into `arena`. Split out so tests can feed a +/// synthetic `[]Change` without running the git/IO pipeline. +/// +/// Cash-destination transfers don't flip their original `new_cash` / +/// `cash_contribution` Change's kind (a single cash delta can be +/// drained by multiple records, which the kind field can't +/// represent). Instead the matcher accumulates the per-record +/// attribution onto each consumed Change's `transfer_attributed` +/// field, in iteration order. `attributedValue()` then returns the +/// residual: 0 for fully-attributed lots, the unattributed +/// remainder for partials. +/// /// Deliberately excludes `partial_transfer_in`. A partial lot already /// has an explicit transfer record acknowledging the large movement; /// the unmatched residual is typically small (pre-existing cash that @@ -952,7 +1028,13 @@ fn collectUnmatchedLargeLots( else => false, }; if (!is_new_side) continue; - if (c.value() < threshold) continue; + + // Use attributedValue() so a fully-attributed cash lot + // (whose `transfer_attributed` covers the whole `value()`) + // drops out, and a partially-attributed cash lot surfaces + // only on the unattributed remainder. + 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. @@ -963,7 +1045,7 @@ fn collectUnmatchedLargeLots( .account = account_copy, .symbol = symbol_copy, .security_type = c.security_type, - .value = c.value(), + .value = residual, .open_date = od, }); } @@ -993,23 +1075,15 @@ fn summarizeAttribution(ctx: ReportContext) AttributionSummary { var new_contributions: f64 = 0; var drip: f64 = 0; for (ctx.report.changes) |c| switch (c.kind) { - .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => new_contributions += c.value(), + .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => new_contributions += c.attributedValue(), .new_drip_lot, .drip_confirmed, .rollup_delta => drip += c.value(), .partial_transfer_in => new_contributions += c.attributedValue(), else => {}, }; - - // Subtract cash-dest transfer attribution. The matcher populates - // `cash_attributed_by_account` with the per-account total of cash - // transfers; those amounts already flowed into `new_contributions` - // above via their `new_cash` / `cash_contribution` Change and need - // to be taken back out. Single-summed subtraction is safe because - // the matcher's budget check guarantees the bucket doesn't exceed - // the pool of positive cash Changes on each account. - var cait = ctx.report.cash_attributed_by_account.iterator(); - while (cait.next()) |entry| { - new_contributions -= entry.value_ptr.*; - } + // Note: cash-dest transfer attribution is already removed by + // `attributedValue()` on the per-Change side (matchCashDestination + // accumulates into `transfer_attributed`). No second subtraction + // off `cash_attributed_by_account` needed here. return .{ .new_contributions = new_contributions, .drip = drip }; } @@ -1130,12 +1204,19 @@ const Change = struct { /// Portion of `value()` that counts toward attribution (after /// transfer reclassification). For `transfer_in` / `transfer_out` /// / `unmatched_transfer`, the contribution is $0. For - /// `partial_transfer_in`, only the residual counts. Everyone else - /// sees `value()` unchanged. + /// `partial_transfer_in`, only the residual counts. For + /// `new_cash`, `cash_contribution`, and `cash_delta`, the + /// matcher may have credited some of the value to a cash- + /// destination transfer record (see `matchCashDestination`); + /// `transfer_attributed` tracks how much. The residual is + /// `value() − transfer_attributed`, which is what shows up in + /// "New contributions / purchases" and the audit large-lot + /// filter. Everyone else sees `value()` unchanged. pub fn attributedValue(self: Change) f64 { return switch (self.kind) { .transfer_in, .transfer_out, .unmatched_transfer => 0, .partial_transfer_in => self.value() - self.transfer_attributed, + .new_cash, .cash_contribution, .cash_delta => self.value() - self.transfer_attributed, else => self.value(), }; } @@ -1441,20 +1522,23 @@ const ReportOptions = struct { /// `cash_contribution` so every downstream consumer (full /// report, per-account summary, `compare` attribution) agrees. account_map: ?*const analysis.AccountMap = null, - /// Transfer log from `transaction_log.srf`. When present, the - /// `matchTransfers` pass reclassifies destination/source Changes - /// as `transfer_in` / `partial_transfer_in` / `transfer_out` and - /// emits `unmatched_transfer` entries for records that can't be - /// matched. When null, `matchTransfers` is a no-op. - transfer_log: ?*const transaction_log.TransactionLog = null, - /// Window start — inclusive. Only transfer records with - /// `transfer >= window_start` are considered. Null when - /// `transfer_log` is null. - window_start: ?Date = null, - /// Window end — inclusive. Only transfer records with - /// `transfer <= window_end` are considered. Null when - /// `transfer_log` is null. - window_end: ?Date = null, + /// Transfer records to match against this diff. Typically the + /// records that are NEW in the after-side `transaction_log.srf` + /// relative to the before-side (computed by + /// `diffTransferLogs`). The matcher reclassifies + /// destination/source Changes as `transfer_in` / + /// `partial_transfer_in` / `transfer_out` and emits + /// `unmatched_transfer` entries for records that can't be + /// matched. When null or empty, `matchTransfers` is a no-op. + /// + /// Records are not date-window filtered: the caller is + /// responsible for passing only records that should be + /// considered for THIS diff. The expected source is "records + /// added to `transaction_log.srf` since `before_rev`," not + /// "records dated within the diff's git timestamp window." + /// See `diffTransferLogs` and the prepareReport pipeline for + /// how the production caller assembles the slice. + transfer_log: ?[]const transaction_log.TransferRecord = null, }; fn computeReport( @@ -1670,20 +1754,19 @@ fn computeReport( } // Transfer reclassification pass: rewrite destination/source - // Change kinds for records in `transaction_log.srf` that match - // the window, and accumulate per-account cash attribution so - // transferred cash doesn't double-count in per-account totals. - // No-op when no transfer_log is supplied. See `matchTransfers` - // docstring for the matching algorithm. + // Change kinds for records the caller passed in (typically the + // diff between before-side and after-side + // `transaction_log.srf`), and accumulate per-account cash + // attribution so transferred cash doesn't double-count in + // per-account totals. No-op when no records are supplied. See + // `matchTransfers` docstring for the matching algorithm. var cash_attributed_by_account: std.StringHashMap(f64) = .init(allocator); - if (opts.transfer_log) |tlog| { + if (opts.transfer_log) |records| { try matchTransfers( allocator, &changes, &cash_attributed_by_account, - tlog, - opts.window_start, - opts.window_end, + records, ); } @@ -1695,7 +1778,12 @@ fn computeReport( if (!gop.found_existing) gop.value_ptr.* = .{}; switch (c.kind) { .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => { - gop.value_ptr.new_money += c.value(); + // attributedValue() returns the unattributed residual + // for cash-kind Changes (matchCashDestination drained + // `transfer_attributed`); for non-cash kinds it equals + // value(). Either way, this is "real new money on the + // account." + gop.value_ptr.new_money += c.attributedValue(); }, .new_drip_lot, .drip_confirmed => { gop.value_ptr.drip_confirmed += c.value(); @@ -1704,7 +1792,7 @@ fn computeReport( gop.value_ptr.rollup += c.value(); }, .cash_delta => { - gop.value_ptr.cash_delta += c.value(); + gop.value_ptr.cash_delta += c.attributedValue(); }, .cd_matured => { // Interest is computed lazily against cash_delta in print(); @@ -1726,37 +1814,13 @@ fn computeReport( } } - // Subtract cash-dest transfer attribution from each account's - // cash-side totals so transferred cash doesn't inflate - // new_money / cash_delta / cash_contribution. The bucket was - // populated by the cash-dest matcher; each account's entry is - // the sum of matched record amounts landing on that account. - // - // Apply to new_money (new_cash contributed there) and - // cash_delta (pooled unclassified); we don't have visibility - // here into WHICH of those buckets held the attributed dollars, - // so take from new_money first (where cash_contribution lands) - // then cash_delta. This matches how `summarizeAttribution` - // handles the same subtraction at the totals level. - var cait = cash_attributed_by_account.iterator(); - while (cait.next()) |entry| { - const acct = entry.key_ptr.*; - var remaining = entry.value_ptr.*; - if (acct_totals.getPtr(acct)) |at| { - const from_new_money = @min(at.new_money, remaining); - at.new_money -= from_new_money; - remaining -= from_new_money; - if (remaining > 0) { - const from_cash_delta = @min(at.cash_delta, remaining); - at.cash_delta -= from_cash_delta; - remaining -= from_cash_delta; - } - // Any leftover `remaining` means the attribution bucket - // exceeded both buckets — shouldn't happen if the - // matcher's budget check holds, but harmless if it does - // (the overage just doesn't land anywhere negative). - } - } + // Note: cash-dest transfer attribution is already removed by + // `attributedValue()` on each cash-side Change (the matcher + // accumulates into `transfer_attributed`). No second subtraction + // off `cash_attributed_by_account` needed here. The bucket is + // still populated for downstream consumers (e.g. callers that + // want a per-account view of attributed transfers) but isn't + // used in the totals math. return .{ .changes = try changes.toOwnedSlice(allocator), @@ -1777,6 +1841,43 @@ fn computeReport( /// dollars for cash lots) without masking real discrepancies. const transfer_amount_tolerance: f64 = 1.0; +/// Compute the slice of transfer records that are NEW in `after` +/// relative to `before` — i.e. records the matcher should consider +/// for the current diff. Records present in both logs are skipped +/// (they paired in their own diff cycle and shouldn't re-match). +/// +/// `before` may be null (e.g. `transaction_log.srf` did not exist in +/// the before-side revision); in that case every record in `after` +/// is treated as new. +/// +/// The returned slice is allocated in `arena` and its records +/// borrow from `after`'s record memory. Treat the slice as read-only +/// and tied to `after`'s lifetime. +/// +/// Equality is `TransferRecord.eql` (total-field equality including +/// optional `note`). User edits to an existing record's any field +/// produce a "new" record from this function's POV; the matcher +/// then re-attempts pairing. See the `eql` doc comment for rationale. +fn diffTransferLogs( + arena: std.mem.Allocator, + before: ?*const transaction_log.TransactionLog, + after: *const transaction_log.TransactionLog, +) ![]const transaction_log.TransferRecord { + var out: std.ArrayList(transaction_log.TransferRecord) = .empty; + errdefer out.deinit(arena); + for (after.transfers) |a| { + var found = false; + if (before) |b| for (b.transfers) |bef| { + if (a.eql(bef)) { + found = true; + break; + } + }; + if (!found) try out.append(arena, a); + } + return try out.toOwnedSlice(arena); +} + /// Reclassify Changes whose lot (or cash_delta) corresponds to a /// recorded transfer. Walks `report.changes` and the in-window /// transfer records together: @@ -1812,9 +1913,14 @@ const transfer_amount_tolerance: f64 = 1.0; /// destination mismatches surface as unmatched_transfer. /// /// Records outside the window (`transfer < window_start` or -/// `transfer > window_end`) are silently skipped — they belong to a -/// different diff. When either window endpoint is null, that side -/// is unbounded. +/// `transfer > window_end`) are NOT filtered here. The caller is +/// responsible for narrowing the slice to records that should be +/// considered for THIS diff. The production path uses +/// `diffTransferLogs` to pass only the records added to +/// `transaction_log.srf` since `before_rev`, regardless of their +/// `transfer::DATE`. This allows a user to back-date a record +/// (e.g. add a `transfer::2026-05-20` entry on 2026-05-23) and +/// have it pair against the working-copy diff that introduced it. /// /// Populates `cash_attributed_by_account` (caller-owned) with the /// per-account total of amounts matched to cash-destination records; @@ -1824,9 +1930,7 @@ fn matchTransfers( allocator: std.mem.Allocator, changes: *std.ArrayList(Change), cash_attributed_by_account: *std.StringHashMap(f64), - tlog: *const transaction_log.TransactionLog, - window_start: ?Date, - window_end: ?Date, + records: []const transaction_log.TransferRecord, ) !void { // Bookkeeping: track which Change indices have already been // claimed by a transfer record to catch duplicates on the @@ -1857,14 +1961,10 @@ fn matchTransfers( } } - // Walk transfer records in file order. - const records = try tlog.transfersInWindow( - allocator, - window_start orelse Date{ .days = std.math.minInt(i32) }, - window_end orelse Date{ .days = std.math.maxInt(i32) }, - ); - defer allocator.free(records); - + // Walk transfer records in caller-supplied order. Caller is + // responsible for filtering to only those records that should + // be considered for THIS diff (typically: records new in the + // after-side `transaction_log.srf` vs. the before-side). for (records) |rec| { if (rec.type == .in_kind) { try appendUnmatched(allocator, changes, rec, "in-kind transfers not yet supported in v1"); @@ -2069,14 +2169,49 @@ fn matchCashDestination( if (!gop.found_existing) gop.value_ptr.* = 0; gop.value_ptr.* += rec.amount; + // Distribute the record amount across the destination account's + // cash-side Changes by accumulating into each Change's + // `transfer_attributed`. We deliberately do NOT flip the + // Change's `kind`: a single cash Change can be drained by + // multiple records (e.g. two transfers landing on the same + // cash lot), and a single record can drain across multiple + // cash Changes (when the user split the inflow into several + // lots). The kind field can only hold one classification. + // + // Bumping `transfer_attributed` makes `attributedValue()` + // return the unattributed residual — fully-attributed Changes + // drop out of the "New contributions" section, audit's + // "Large new lots" filter, and any other consumer that asks + // "how much of this Change is real new money?". The summary + // 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; + for (changes.items) |*c| { + if (remaining <= 0) break; + if (!std.mem.eql(u8, c.account, rec.to)) continue; + const is_cash_kind = switch (c.kind) { + .new_cash, .cash_contribution => true, + .cash_delta => c.value() > 0, + else => false, + }; + if (!is_cash_kind) continue; + const unattributed = c.value() - c.transfer_attributed; + if (unattributed <= 0) continue; + const draw = @min(unattributed, remaining); + c.transfer_attributed += draw; + remaining -= draw; + } + // `remaining > 0` here would indicate the budget pre-check + // accepted a record we couldn't actually distribute. Possible + // if the cash-side Changes' `value()` totals don't agree with + // the `cash_budget` we built (they should — both are derived + // from the same Changes). Leave it as silent over-credit on + // the per-account bucket; the user-visible symptom would be + // a small Joint-trust-style residual showing up in audit. + // Append a synthetic transfer_in Change for Transfers-section - // display. We deliberately do NOT flip an existing cash - // Change's kind: a single cash_delta can be drained by multiple - // records, and the kind field can only hold one classification. - // Keeping the original cash Change intact preserves its - // appearance in the "Cash deltas" section of the report; the - // per-account totals reconciliation handles the attribution - // math via `cash_attributed_by_account`. + // display. const from = try allocator.dupe(u8, rec.from); const to = try allocator.dupe(u8, rec.to); const note_copy: ?[]const u8 = if (rec.note) |n| try allocator.dupe(u8, n) else null; @@ -2149,8 +2284,14 @@ fn printReport(out: *std.Io.Writer, report: *const Report, label: []const u8, co var new_total: f64 = 0; for (report.changes) |c| switch (c.kind) { .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => { + // Use attributedValue so a cash lot fully covered by a + // transfer record (whose `transfer_attributed` equals + // its `value()`) drops out, and a partially-covered + // lot shows only the unattributed remainder. + const residual = c.attributedValue(); + if (residual <= 0) continue; any = true; - new_total += c.value(); + new_total += residual; try printChangeLine(out, c, color, pos_color); }, .partial_transfer_in => { @@ -3694,6 +3835,140 @@ test "short: short input returned as-is" { // `ReportOptions`, and asserts the resulting `Report.changes` kinds // and per-account attribution. +test "diffTransferLogs: empty before, all records new" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const after = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\transfer::2026-05-03,type::cash,amount:num:1000,from::Acct C,to::Acct D,dest_lot::cash + \\ + ); + + const new_records = try diffTransferLogs(arena, null, &after); + try std.testing.expectEqual(@as(usize, 2), new_records.len); +} + +test "diffTransferLogs: identical before and after returns empty" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const before = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + const after = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + + const new_records = try diffTransferLogs(arena, &before, &after); + try std.testing.expectEqual(@as(usize, 0), new_records.len); +} + +test "diffTransferLogs: only the new record returned" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const before = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + const after = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\transfer::2026-05-03,type::cash,amount:num:1000,from::Acct C,to::Acct D,dest_lot::cash + \\ + ); + + const new_records = try diffTransferLogs(arena, &before, &after); + try std.testing.expectEqual(@as(usize, 1), new_records.len); + try std.testing.expectEqual(Date.fromYmd(2026, 5, 3).days, new_records[0].transfer.days); +} + +test "diffTransferLogs: edited record treated as new" { + // User changed the `from` field on a previously-recorded transfer. + // Old form is in `before`, new form is in `after`. The new form + // doesn't equal anything in before → returned as new. The old + // form is in before but not after → silently dropped (its diff + // cycle is over). + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const before = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Wrong Acct,to::Acct B,dest_lot::cash + \\ + ); + const after = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + + const new_records = try diffTransferLogs(arena, &before, &after); + try std.testing.expectEqual(@as(usize, 1), new_records.len); + try std.testing.expectEqualStrings("Acct A", new_records[0].from); +} + +test "diffTransferLogs: out-of-order records still match correctly" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const before = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\transfer::2026-05-03,type::cash,amount:num:1000,from::Acct C,to::Acct D,dest_lot::cash + \\ + ); + // After has the same two records but in reverse order. + const after = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-03,type::cash,amount:num:1000,from::Acct C,to::Acct D,dest_lot::cash + \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + + const new_records = try diffTransferLogs(arena, &before, &after); + try std.testing.expectEqual(@as(usize, 0), new_records.len); +} + +test "diffTransferLogs: back-dated record added on top of existing log" { + // Regression test for the original bug: user records a transfer + // months after it actually happened. The before-side log has + // some prior records; the after-side adds one with an old + // transfer::DATE. That new record must be returned even though + // its date predates everything in before. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const before = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-15,type::cash,amount:num:1000,from::Acct A,to::Acct B,dest_lot::cash + \\ + ); + const after = try transaction_log.parseTransactionLogFile(arena, + \\#!srfv1 + \\transfer::2026-05-15,type::cash,amount:num:1000,from::Acct A,to::Acct B,dest_lot::cash + \\transfer::2026-01-15,type::cash,amount:num:9999,from::Old Acct,to::New Acct,dest_lot::cash + \\ + ); + + const new_records = try diffTransferLogs(arena, &before, &after); + try std.testing.expectEqual(@as(usize, 1), new_records.len); + try std.testing.expectEqual(@as(f64, 9999), new_records[0].amount); +} + test "matchTransfers: cash-to-cash happy path" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); @@ -3707,16 +3982,14 @@ test "matchTransfers: cash-to-cash happy path" { .{ .symbol = "cash", .shares = 5000, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:5000,from::Acct A,to::Acct B,dest_lot::cash \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 3), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // The new_cash Change stays as new_cash in the display (we don't @@ -3754,16 +4027,14 @@ test "matchTransfers: lot destination happy path" { .{ .symbol = "SYM", .shares = 80, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 100, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); try std.testing.expectEqual(@as(usize, 1), report.changes.len); @@ -3789,16 +4060,14 @@ test "matchTransfers: partial attribution — transfer smaller than lot" { .{ .symbol = "SYM", .shares = 80, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 100, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:7000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); try std.testing.expectEqual(@as(usize, 1), report.changes.len); @@ -3826,7 +4095,7 @@ test "matchTransfers: sweep — lot destination + cash residual, both match" { .{ .symbol = "cash", .shares = 4700, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:145300,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\transfer::2026-05-02,type::cash,amount:num:4700,from::Acct A,to::Acct B,dest_lot::cash @@ -3834,9 +4103,7 @@ test "matchTransfers: sweep — lot destination + cash residual, both match" { ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // Acct B new_money: $0 (both transfers fully attributed). @@ -3873,7 +4140,7 @@ test "matchTransfers: duplicate dest_lot emits unmatched" { // Two records pointing at the same (account, symbol). First matches; // second emits unmatched_transfer with "already claimed". - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\transfer::2026-05-02,type::cash,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 @@ -3881,9 +4148,7 @@ test "matchTransfers: duplicate dest_lot emits unmatched" { ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); var n_transfer_in: usize = 0; @@ -3908,16 +4173,14 @@ test "matchTransfers: missing dest_lot emits unmatched" { const before = [_]Lot{}; const after = [_]Lot{}; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); try std.testing.expectEqual(@as(usize, 1), report.changes.len); @@ -3938,16 +4201,14 @@ test "matchTransfers: amount exceeds lot value emits unmatched" { }; // $10k transfer against $8k lot. - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:10000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // Lot stays as new_stock; unmatched record appended. @@ -3975,16 +4236,14 @@ test "matchTransfers: cash insufficient emits unmatched" { .{ .symbol = "cash", .shares = 3000, .open_date = Date.fromYmd(2026, 5, 2), .open_price = 1.0, .security_type = .cash, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:5000,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, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); var n_unmatched: usize = 0; @@ -4011,7 +4270,7 @@ test "matchTransfers: same-day multi-cash records drain a single cash_delta" { }; // Two records ($2k + $3k = $5k). - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:2000,from::Acct A,to::Acct B,dest_lot::cash \\transfer::2026-05-02,type::cash,amount:num:3000,from::Acct A,to::Acct B,dest_lot::cash @@ -4019,9 +4278,7 @@ test "matchTransfers: same-day multi-cash records drain a single cash_delta" { ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // Both records should match (budget fully consumed); no unmatched. @@ -4054,16 +4311,14 @@ test "matchTransfers: type::in_kind always emits unmatched" { .{ .symbol = "SYM", .shares = 80, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 100, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::in_kind,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // new_stock stays as new_stock (not flipped); unmatched appended. @@ -4078,7 +4333,17 @@ test "matchTransfers: type::in_kind always emits unmatched" { try std.testing.expectEqual(@as(usize, 1), n_unmatched); } -test "matchTransfers: transfer outside window is ignored" { +test "matchTransfers: back-dated record matches regardless of date" { + // The matcher itself is date-agnostic now — the caller (typically + // `prepareReport` via `diffTransferLogs`) is responsible for + // narrowing the slice to records that should be considered for + // this diff cycle. A record dated weeks before any "diff window" + // pairs cleanly when passed to the matcher directly. + // + // This is the regression test for the back-dated-record-rejected + // bug: user adds `transfer::2026-01-15` to transaction_log.srf + // on 2026-05-04 to record a transfer that actually happened + // months ago. The matcher must pair it. var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const allocator = arena_state.allocator(); @@ -4091,22 +4356,19 @@ test "matchTransfers: transfer outside window is ignored" { .{ .symbol = "SYM", .shares = 80, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 100, .account = "Acct B" }, }; - // Record is before the window start — should be silently skipped. - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-01-15,type::cash,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 4, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); - // Record skipped → lot stays new_stock, no unmatched. + // Record paired despite the months-earlier date. try std.testing.expectEqual(@as(usize, 1), report.changes.len); - try std.testing.expectEqual(ChangeKind.new_stock, report.changes[0].kind); + try std.testing.expectEqual(ChangeKind.transfer_in, report.changes[0].kind); } test "matchTransfers: null transfer_log is a no-op (backward compat)" { @@ -4145,16 +4407,14 @@ test "matchTransfers: attribution excludes transferred amount" { .{ .symbol = "SYM", .shares = 80, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 100, .account = "Acct B" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:8000,from::Acct A,to::Acct B,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // Replicate summarizeAttribution's logic directly. @@ -4260,16 +4520,14 @@ test "collectUnmatchedLargeLots: matched via transfer log is silent" { }; // Transfer log fully covers the lot → kind flips to transfer_in. - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:50000,from::Acct B,to::Acct A,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); // Sanity: the lot should have been reclassified. @@ -4279,6 +4537,115 @@ test "collectUnmatchedLargeLots: matched via transfer log is silent" { try std.testing.expectEqual(@as(usize, 0), lots.len); } +test "collectUnmatchedLargeLots: cash-destination matched is silent" { + // Regression for the user-visible bug: a $73,158.33 cash lot on + // Joint trust funded by a transfer record dated 2026-05-20 was + // surfacing in audit's "Large new lots — confirm source" because + // the cash matcher doesn't flip the original `new_cash` Change's + // kind (it draws from `cash_attributed_by_account` instead). + // Without subtracting that attribution, the audit filter + // re-flagged a lot that's already explained. + 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{}; + const after = [_]Lot{ + .{ .security_type = .cash, .shares = 73158.33, .open_date = Date.fromYmd(2026, 5, 20), .open_price = 1.0, .account = "Joint trust" }, + }; + + const tlog = try transaction_log.parseTransactionLogFile(allocator, + \\#!srfv1 + \\transfer::2026-05-20,type::cash,amount:num:73158.33,from::Fidelity Emil,to::Joint trust,dest_lot::cash + \\ + ); + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 23), .{ + .transfer_log = tlog.transfers, + }); + + // Cash-dest matching does NOT flip the original new_cash Change + // (a single delta can be drained by multiple records). The + // matcher records the attributed amount in + // `cash_attributed_by_account` instead. + var saw_new_cash = false; + var saw_synthetic_transfer = false; + for (report.changes) |c| switch (c.kind) { + .new_cash => saw_new_cash = true, + .transfer_in => saw_synthetic_transfer = true, + else => {}, + }; + try std.testing.expect(saw_new_cash); + try std.testing.expect(saw_synthetic_transfer); + const attributed = report.cash_attributed_by_account.get("Joint trust") orelse 0; + try std.testing.expectEqual(@as(f64, 73158.33), attributed); + + // The audit filter must subtract the attribution and stay quiet. + const lots = try collectUnmatchedLargeLots(allocator, report.changes, 10_000.0); + try std.testing.expectEqual(@as(usize, 0), lots.len); +} + +test "collectUnmatchedLargeLots: cash-destination partial match surfaces residual only" { + // A $50K cash lot with a $30K transfer attributed against it — + // the residual $20K is "new contribution" and SHOULD surface + // (above the $10K threshold). The filter reports the residual, + // not the gross. + 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{}; + const after = [_]Lot{ + .{ .security_type = .cash, .shares = 50000.0, .open_date = Date.fromYmd(2026, 5, 20), .open_price = 1.0, .account = "Joint trust" }, + }; + + const tlog = try transaction_log.parseTransactionLogFile(allocator, + \\#!srfv1 + \\transfer::2026-05-20,type::cash,amount:num:30000,from::Fidelity Emil,to::Joint trust,dest_lot::cash + \\ + ); + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 23), .{ + .transfer_log = tlog.transfers, + }); + + const lots = try collectUnmatchedLargeLots(allocator, report.changes, 10_000.0); + try std.testing.expectEqual(@as(usize, 1), lots.len); + try std.testing.expectEqual(@as(f64, 20000.0), lots[0].value); +} + +test "collectUnmatchedLargeLots: cash-destination partial below threshold is silent" { + // A $15K cash lot with a $10K transfer attributed → residual + // $5K, below the $10K threshold → silent. + 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{}; + const after = [_]Lot{ + .{ .security_type = .cash, .shares = 15000.0, .open_date = Date.fromYmd(2026, 5, 20), .open_price = 1.0, .account = "Joint trust" }, + }; + + const tlog = try transaction_log.parseTransactionLogFile(allocator, + \\#!srfv1 + \\transfer::2026-05-20,type::cash,amount:num:10000,from::Fidelity Emil,to::Joint trust,dest_lot::cash + \\ + ); + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 23), .{ + .transfer_log = tlog.transfers, + }); + + const lots = try collectUnmatchedLargeLots(allocator, report.changes, 10_000.0); + try std.testing.expectEqual(@as(usize, 0), lots.len); +} + test "collectUnmatchedLargeLots: no new lots → empty result" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); @@ -4317,16 +4684,14 @@ test "collectUnmatchedLargeLots: partial transfer still flags residual? No — f .{ .symbol = "SYM", .shares = 100, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 500, .account = "Acct A" }, }; - var tlog = try transaction_log.parseTransactionLogFile(allocator, + const tlog = try transaction_log.parseTransactionLogFile(allocator, \\#!srfv1 \\transfer::2026-05-02,type::cash,amount:num:45000,from::Acct B,to::Acct A,dest_lot::SYM@2026-05-03 \\ ); const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ - .transfer_log = &tlog, - .window_start = Date.fromYmd(2026, 1, 1), - .window_end = Date.fromYmd(2026, 12, 31), + .transfer_log = tlog.transfers, }); try std.testing.expectEqual(ChangeKind.partial_transfer_in, report.changes[0].kind); diff --git a/src/models/transaction_log.zig b/src/models/transaction_log.zig index e41ba3b..dff4c41 100644 --- a/src/models/transaction_log.zig +++ b/src/models/transaction_log.zig @@ -115,7 +115,7 @@ pub const DestLot = union(enum) { defer allocator.free(buf); var out = try allocator.alloc(u8, buf.len + 10); @memcpy(out[0..buf.len], buf); - _ = std.fmt.bufPrint(out[buf.len..][0..10], "{f}", .{l.open_date}) catch unreachable; + _ = try std.fmt.bufPrint(out[buf.len..][0..10], "{f}", .{l.open_date}); break :blk .{ .string = out }; }, }; @@ -152,6 +152,34 @@ pub const TransferRecord = struct { to: []const u8, dest_lot: DestLot, note: ?[]const u8 = null, + + /// Total-field equality. Used by the contributions matcher to + /// identify records that already existed in the before-side + /// `transaction_log.srf` (and therefore already paired in a + /// previous diff cycle). + /// + /// Any field difference — including the optional `note` — + /// produces a non-equal result. This treats "user edited a + /// previously-recorded transfer" as a new record for matching + /// purposes; if the edit doesn't correspond to a fresh + /// portfolio change it surfaces as `unmatched_transfer` in the + /// Flagged section, which is the correct user-visible signal. + /// + /// `amount` uses exact f64 equality. Records are user-authored + /// and rounded; any auto-generated record that round-trips + /// through f64 differently would need a tolerance, but no + /// current caller produces those. + pub fn eql(a: TransferRecord, b: TransferRecord) bool { + if (a.transfer.days != b.transfer.days) return false; + if (a.type != b.type) return false; + if (a.amount != b.amount) return false; + if (!std.mem.eql(u8, a.from, b.from)) return false; + if (!std.mem.eql(u8, a.to, b.to)) return false; + if (!a.dest_lot.eql(b.dest_lot)) return false; + if (a.note == null and b.note == null) return true; + if (a.note == null or b.note == null) return false; + return std.mem.eql(u8, a.note.?, b.note.?); + } }; /// Parsed transaction log. `transfers` is allocator-owned; all string @@ -370,6 +398,158 @@ test "DestLot.eql: different date" { try testing.expect(!a.eql(b)); } +test "TransferRecord.eql: identical records" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .type = .cash, + .amount = 73158.0, + .from = "Fidelity Emil", + .to = "Sample Trust", + .dest_lot = .cash, + .note = null, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .type = .cash, + .amount = 73158.0, + .from = "Fidelity Emil", + .to = "Sample Trust", + .dest_lot = .cash, + .note = null, + }; + try testing.expect(a.eql(b)); +} + +test "TransferRecord.eql: different date" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 21), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + try testing.expect(!a.eql(b)); +} + +test "TransferRecord.eql: different amount" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100.01, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + try testing.expect(!a.eql(b)); +} + +test "TransferRecord.eql: different from" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A2", + .to = "B", + .dest_lot = .cash, + }; + try testing.expect(!a.eql(b)); +} + +test "TransferRecord.eql: different dest_lot" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .{ .lot = .{ .symbol = "AMZN", .open_date = Date.fromYmd(2026, 5, 20) } }, + }; + try testing.expect(!a.eql(b)); +} + +test "TransferRecord.eql: note difference treated as different" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + .note = "v1", + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + .note = "v2", + }; + try testing.expect(!a.eql(b)); +} + +test "TransferRecord.eql: both notes null treated as equal" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + }; + try testing.expect(a.eql(b)); +} + +test "TransferRecord.eql: one note null other set treated as different" { + const a: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + .note = null, + }; + const b: TransferRecord = .{ + .transfer = Date.fromYmd(2026, 5, 20), + .amount = 100, + .from = "A", + .to = "B", + .dest_lot = .cash, + .note = "x", + }; + try testing.expect(!a.eql(b)); +} + test "parseTransactionLogFile: empty file" { var log = try parseTransactionLogFile(testing.allocator, \\#!srfv1