remove dead code

src/commands/contributions.zig

@@ -177,9 +177,12 @@
 //! `new_cash` Change (a single cash delta can be drained by
 //! multiple records, which `kind` can't represent). Instead the
 //! matcher appends a synthetic `transfer_in` Change for the
-//! Transfers section and records the attributed amount in
+//! Transfers section and accumulates the attributed amount onto each
-//! `Report.cash_attributed_by_account`, which the per-account totals
+//! consumed Change's `transfer_attributed` field. `attributedValue()`
-//! and attribution summary subtract from cash-side contributions.
+//! then reports the unattributed residual, which is what the
+//! per-account totals, the contributions sections, and audit's
+//! large-lot filter all consume - one mechanism, so no two views of
+//! the same attribution can drift apart.
 //!
 //! ### Which records does the matcher consider?
 //!
@@ -1036,28 +1039,6 @@ pub fn findUnmatchedLargeLots(
     return .{ .lots = lots, .arena = arena_state };
 }
 
-/// Pure filter: pick out new-side Changes with `value() >= 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 (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
@@ -1131,8 +1112,6 @@ fn summarizeAttribution(ctx: ReportContext) AttributionSummary {
     //                        + cash_contribution (opt-in cash_delta)
     //                        + partial_transfer_in residual
     //                          (`value()` - `transfer_attributed`)
-    //                        - cash-dest transfer totals
-    //                          (from `cash_attributed_by_account`)
     //   - DRIP: new_drip_lot + drip_confirmed + rollup_delta
     // `rollup_delta` is the ambiguous "share increased on a drip::false
     // lot" case. Lumping it with DRIP here matches the report's own
@@ -1152,10 +1131,10 @@ fn summarizeAttribution(ctx: ReportContext) AttributionSummary {
         .partial_transfer_in => new_contributions += c.attributedValue(),
         else => {},
     };
-    // Note: cash-dest transfer attribution is already removed by
+    // Cash-dest transfer attribution is already removed by `attributedValue()` on
-    // `attributedValue()` on the per-Change side (matchCashDestination
+    // the per-Change side: `matchCashDestination` accumulates into
-    // accumulates into `transfer_attributed`). No second subtraction
+    // `transfer_attributed`, so a fully-attributed cash Change contributes zero
-    // off `cash_attributed_by_account` needed here.
+    // here without any separate per-account subtraction.
 
     return .{ .new_contributions = new_contributions, .drip = drip };
 }
@@ -1329,11 +1308,6 @@ const Report = struct {
     changes: []Change,
     /// Per-account rollups for the summary section.
     account_totals: std.StringHashMap(AccountTotal),
-    /// Per-account cash amounts matched to transfer records. Subtracted
-    /// from cash-side totals in the per-account summary so transferred
-    /// cash doesn't double-count. Keys borrow from Change.account
-    /// strings (arena-owned, same lifetime as the Report).
-    cash_attributed_by_account: std.StringHashMap(f64),
 
     const AccountTotal = struct {
         new_money: f64 = 0, // stock+cd+cash new lots (drip::false)
@@ -1873,18 +1847,10 @@ fn computeReport(
     // Transfer reclassification pass: rewrite destination/source
     // 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
+    // `transaction_log.srf`). No-op when no records are supplied. See
-    // 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) |records| {
-        try matchTransfers(
+        try matchTransfers(allocator, &changes, records);
-            allocator,
-            &changes,
-            &cash_attributed_by_account,
-            records,
-        );
     }
 
     // Intra-account purchase netting: a decrease in an account's cash
@@ -1941,18 +1907,13 @@ fn computeReport(
         }
     }
 
-    // Note: cash-dest transfer attribution is already removed by
+    // Cash-dest transfer attribution is already removed by `attributedValue()` on
-    // `attributedValue()` on each cash-side Change (the matcher
+    // each cash-side Change - the matcher accumulates into `transfer_attributed`,
-    // accumulates into `transfer_attributed`). No second subtraction
+    // so the residual is what reaches these totals. Nothing further to subtract.
-    // 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),
         .account_totals = acct_totals,
-        .cash_attributed_by_account = cash_attributed_by_account,
     };
 }
 
@@ -2042,10 +2003,10 @@ fn diffTransferLogs(
 ///       cash_contribution summed) can cover the record's amount
 ///       (minus any prior cash-dest records on the same account).
 ///       Success appends a synthetic `transfer_in` Change for
-///       display AND accumulates into
+///       display AND accumulates onto the consumed cash Changes'
-///       `cash_attributed_by_account[to]`, which the caller
+///       `transfer_attributed`, so `attributedValue()` reports only
-///       subtracts from cash-side per-account totals. Failure
+///       the unattributed residual. Failure (budget underflow) emits
-///       (budget underflow) emits `unmatched_transfer`.
+///       `unmatched_transfer`.
 ///
 ///   - For the `from` side: try to find a matching negative
 ///     `cash_delta` or `lot_removed` on the sending account and
@@ -2065,15 +2026,9 @@ fn diffTransferLogs(
 /// `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;
-/// these amounts are subtracted from the cash bucket in the
-/// per-account totals pass so transferred cash doesn't double-count.
 fn matchTransfers(
     allocator: std.mem.Allocator,
     changes: *std.ArrayList(Change),
-    cash_attributed_by_account: *std.StringHashMap(f64),
     records: []const transaction_log.TransferRecord,
 ) !void {
     // Bookkeeping: track which Change indices have already been
@@ -2132,7 +2087,7 @@ fn matchTransfers(
                 try matchLotDestination(allocator, changes, &consumed_lot_idx, rec, dl);
             },
             .cash => {
-                try matchCashDestination(allocator, changes, &cash_budget, cash_attributed_by_account, &transfer_funding, rec);
+                try matchCashDestination(allocator, changes, &cash_budget, &transfer_funding, rec);
             },
         }
 
@@ -2295,14 +2250,17 @@ fn appendUnmatchedWithOwnedNote(
 
 /// Verify the `to` account's cash budget has capacity for this
 /// record, draw from it, and either attach to an existing cash
-/// Change or append a synthetic one. The per-account attribution
+/// Change or append a synthetic one.
-/// bucket (`cash_attributed_by_account`) is what actually drives
+///
-/// totals math - the Change-level reclassification is for display.
+/// Attribution is recorded per-Change on `transfer_attributed`, which
+/// `attributedValue()` turns into the unattributed residual. That single
+/// mechanism drives every consumer - the totals math, the contributions
+/// sections, and audit's large-lot filter - so there is no separate
+/// per-account bucket to keep in agreement with it.
 fn matchCashDestination(
     allocator: std.mem.Allocator,
     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 {
@@ -2333,15 +2291,6 @@ fn matchCashDestination(
     // capacity.
     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;
-    // 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
     // `transfer_attributed`. We deliberately do NOT flip the
@@ -2355,10 +2304,10 @@ fn matchCashDestination(
     // 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
+    // "how much of this Change is real new money?". Every consumer -
-    // pass continues to use `cash_attributed_by_account` for its
+    // per-account totals, the contributions sections, audit's large-lot
-    // per-account math - the two views agree because the same
+    // filter - reads that same residual, so there is no second view to
-    // amount is subtracted on both sides.
+    // keep in agreement.
     var remaining = credited;
     for (changes.items) |*c| {
         if (remaining <= 0) break;
@@ -4930,9 +4879,9 @@ test "matchTransfers: cash-to-cash happy path" {
     // The new_cash Change stays as new_cash in the display (we don't
     // flip cash-side Changes because a single cash_delta can be
     // drained by multiple records). A synthetic transfer_in Change
-    // is appended for Transfers-section display, and
+    // is appended for Transfers-section display, and the $5k lands on
-    // `cash_attributed_by_account` carries the $5k subtraction for
+    // the cash Change's `transfer_attributed` so `attributedValue()`
-    // attribution math.
+    // reports the residual.
     var n_new_cash: usize = 0;
     var n_transfer_in: usize = 0;
     for (report.changes) |c| switch (c.kind) {
@@ -5829,17 +5778,21 @@ test "matchTransfers: attribution excludes transferred amount" {
         .transfer_log = tlog.transfers,
     });
 
-    // Replicate summarizeAttribution's logic directly.
+    // Replicate summarizeAttribution's logic directly. Uses
+    // `attributedValue()` on the new-side kinds, exactly as the real
+    // summary does: the per-Change residual IS the subtraction, so there
+    // is nothing further to net off. An earlier version of this test
+    // summed raw `value()` and then subtracted a separate per-account
+    // bucket - the superseded mechanism - which meant it reproduced an
+    // old formula rather than exercising the shipped one.
     var new_contributions: f64 = 0;
     var drip: f64 = 0;
     for (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 => {},
     };
-    var cait = report.cash_attributed_by_account.iterator();
-    while (cait.next()) |entry| new_contributions -= entry.value_ptr.*;
 
     try std.testing.expectApproxEqAbs(@as(f64, 0.0), new_contributions, 0.01);
     try std.testing.expectApproxEqAbs(@as(f64, 0.0), drip, 0.01);
@@ -5994,7 +5947,7 @@ test "collectUnmatchedLargeLots: cash-destination matched is silent" {
     // Sample 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).
+    // kind (the attribution rides on `transfer_attributed` 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);
@@ -6021,7 +5974,7 @@ test "collectUnmatchedLargeLots: cash-destination matched is silent" {
     // 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.
+    // `transfer_attributed` instead.
     var saw_new_cash = false;
     var saw_synthetic_transfer = false;
     for (report.changes) |c| switch (c.kind) {
@@ -6031,10 +5984,16 @@ test "collectUnmatchedLargeLots: cash-destination matched is silent" {
     };
     try std.testing.expect(saw_new_cash);
     try std.testing.expect(saw_synthetic_transfer);
-    const attributed = report.cash_attributed_by_account.get("Sample Trust") orelse 0;
+    // The attribution lands on the consumed cash Change, so the residual - not a
-    try std.testing.expectEqual(@as(f64, 73158.33), attributed);
+    // separate per-account bucket - is what every consumer sees. Assert it there,
+    // which is also the value the audit filter below acts on.
+    var residual: f64 = 0;
+    for (report.changes) |c| {
+        if (c.kind == .new_cash) residual += c.attributedValue();
+    }
+    try std.testing.expectApproxEqAbs(@as(f64, 0.0), residual, 0.01);
 
-    // The audit filter must subtract the attribution and stay quiet.
+    // The audit filter must see a fully-attributed lot and stay quiet.
     const lots = try collectUnmatchedLargeLots(allocator, report.changes, null);
     try std.testing.expectEqual(@as(usize, 0), lots.len);
 }
@@ -6554,13 +6513,10 @@ test "printReport: empty report says no changes" {
     var w: std.Io.Writer = .fixed(&buf);
     var account_totals = std.StringHashMap(Report.AccountTotal).init(testing.allocator);
     defer account_totals.deinit();
-    var cash_attr = std.StringHashMap(f64).init(testing.allocator);
-    defer cash_attr.deinit();
     var changes = [_]Change{};
     const report = Report{
         .changes = changes[0..],
         .account_totals = account_totals,
-        .cash_attributed_by_account = cash_attr,
     };
     try printReport(&w, &report, "portfolio.srf", false);
     const out = w.buffered();
@@ -6599,12 +6555,10 @@ test "printReport: full report renders every section and sub-printer" {
     try account_totals.put("Roth", .{ .new_money = 3800, .drip_confirmed = 475, .rollup = 0, .cash_delta = 0 });
     try account_totals.put("CD Acct", .{ .cash_delta = 10500 });
     try account_totals.put("", .{}); // exercises "(no account)" label + all-zero summary cells
-    const cash_attr = std.StringHashMap(f64).init(arena);
 
     const report = Report{
         .changes = changes[0..],
         .account_totals = account_totals,
-        .cash_attributed_by_account = cash_attr,
     };
 
     var buf: [16384]u8 = undefined;