contributions: support all portfolio files and in account sale/rebuy in same window

docs/guides/track-contributions.md

@@ -73,6 +73,15 @@ naive diff sees the receiving account gain lots and counts it as new
 money. `transaction_log.srf` is how you tell zfin "this was a transfer,
 not new money."
 
+This is only needed for movement **between** accounts. Reallocating
+*within* one account -- selling a holding to buy another -- is detected
+on its own, with the sale's proceeds offsetting the repurchase, so no
+record is required. See
+[Internal movement](../reference/cli/contributions.md#internal-movement).
+Recording `close_price` on the sold lot is worth the keystrokes: it is
+what the sale realized, and zfin values the offset from it rather than
+guessing from the current price.
+
 Like the other sibling files, it's **optional and additive**: you only
 need it if you move money between your own accounts and want clean
 attribution. Without it nothing breaks -- those transfers just show up

docs/reference/cli/contributions.md

@@ -7,9 +7,11 @@ points in your portfolio's **git history**.
 Usage: zfin contributions [opts]
 ```
 
-`contributions` diffs two git revisions of your `portfolio.srf` and
-attributes the share/lot changes to new money vs. market movement. Your
-portfolio must be under git with commits over time.
+`contributions` diffs two git revisions of your `portfolio*.srf` files
+and attributes the share/lot changes to new money vs. market movement.
+Every file matching the glob is read at both revisions and merged, so a
+sold lot archived into a sibling `portfolio_closed.srf` is still seen.
+Your portfolio must be under git with commits over time.
 
 ## Modes
 
@@ -41,9 +43,38 @@ most one of `--until`/`--commit-after`.
 zfin contributions --since 1Y
 ```
 
-Internal transfers between your own accounts are excluded from the
-attribution total when declared in
-[`transaction_log.srf`](../config/transaction-log-srf.md).
+## Internal movement
+
+Money that was already inside an account is not a contribution -- it
+just changed form. Two shapes are detected automatically, with no
+bookkeeping on your part, and both report under **Internal purchases**
+rather than counting toward the total:
+
+- **Buying with cash already in the account.** The buy appears
+  alongside the account's cash going down.
+- **Reallocating -- selling one holding to buy another in the same
+  account.** The sale's proceeds offset the repurchase.
+
+A sale is valued at `close_price` when you record one (see
+[`portfolio.srf`](../config/portfolio-srf.md)), which is what the sale
+actually realized. If you delete the lot outright instead, there is no
+`close_price` to read and the current market price stands in -- accurate
+for a recent sale, less so for one made long before the end of the
+window. The report labels which was used: `at close` or `at mark`.
+
+Closing a position that has accumulated a lot per dividend
+reinvestment retires many lots at once, so sales collapse to one line
+per account and symbol, carrying the lot count and the total.
+
+Proceeds still sitting in cash at the end of the window cannot have
+funded anything, and are treated accordingly. On an account marked
+`cash_is_contribution::true` they also cancel that account's cash
+credit, since the sale is not new money even though cash arrived.
+
+Movement **between** accounts is a different matter -- zfin cannot tell
+it from a contribution, so declare it in
+[`transaction_log.srf`](../config/transaction-log-srf.md). An explicit
+record always wins over the automatic netting above.
 
 ## See also
 

docs/reference/config/portfolio-srf.md

@@ -37,8 +37,8 @@ symbol::VTI,shares:num:100,open_date::2024-01-15,open_price:num:220.50,account::
 | `shares`        | number | Yes      | Share count (or face value for cash/CDs). Negative for short option positions.        |
 | `open_date`     | string | Yes\*\*  | Purchase date `YYYY-MM-DD`. \*\*Not required for `cash`/`watch`.                      |
 | `open_price`    | number | Yes\*\*  | Purchase price per share. \*\*Not required for `cash`/`watch`.                        |
-| `close_date`    | string | No       | Sale date. Omit for an open lot.                                                      |
-| `close_price`   | number | No       | Sale price per share.                                                                 |
+| `close_date`    | string | No       | Sale date. Omit for an open lot. See [Closed lots](#closed-lots).                     |
+| `close_price`   | number | No       | Sale price per share. See [Closed lots](#closed-lots).                                |
 | `security_type` | string | No       | `stock` (default), `option`, `cd`, `cash`, `illiquid`, `watch`.                       |
 | `account`       | string | No       | Account name. Should match an `account::` entry in [`accounts.srf`](accounts-srf.md). |
 | `note`          | string | No       | Free-text note (shown in cash/CD/illiquid tables).                                    |
@@ -96,6 +96,26 @@ security_type::cd,symbol::912797KR0,shares:num:10000,open_date::2024-06-01,open_
 security_type::illiquid,symbol::HOME,shares:num:450000,open_date::2020-06-01,open_price:num:350000,note::Primary residence
 ```
 
+## Closed lots
+
+A lot with `close_date` set is sold. Both fields are optional -- an open
+lot has neither -- but a sold lot should carry both, because they drive
+two things beyond bookkeeping:
+
+- **Realized gain/loss** comes from `close_price` against `open_price`.
+- **[`zfin contributions`](../cli/contributions.md#internal-movement)**
+  treats the close as a sale and values it at `close_price`. That
+  matters when you sell one holding to buy another: the proceeds offset
+  the repurchase instead of it reading as new money. Without a
+  `close_price` the current market price stands in, which is close
+  enough for a recent sale and wrong for an old one.
+
+You can either edit the lot where it sits or move it to a sibling file
+such as `portfolio_closed.srf` -- the `portfolio*.srf` glob picks it up
+either way, so realized gains and back-dated (`--as-of`) views stay
+correct. See
+[splitting your portfolio](../../guides/set-up-your-portfolio.md).
+
 ## Price resolution
 
 For stock lots, the displayed price is resolved in this order:

docs/reference/config/transaction-log-srf.md

@@ -94,7 +94,11 @@ destination is either fully a transfer or not one at all.
 ## Scope and limits
 
 - Only `transfer::` records. Buys, sells, and dividends stay inferred
-  from the portfolio diff.
+  from the portfolio diff. You do **not** need a record for selling one
+  holding to buy another inside a single account -- that is detected
+  automatically, and the sale's proceeds offset the repurchase. See
+  [`zfin contributions`](../cli/contributions.md#internal-movement) for
+  how sales are valued and when a record is still required.
 - Forward-looking only -- there is no historical reconstruction.
 - Account names are matched byte-exactly, so a
   [renamed account](../../guides/set-up-accounts.md#renaming-an-account)

src/commands/audit.zig

@@ -118,13 +118,17 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     const verbose = parsed.verbose;
     const stale_days = parsed.stale_days;
 
-    // Flagless mode: run portfolio hygiene check (single-file
-    // semantics - git blame, commit SHAs, etc.). Resolve paths
-    // just to find the anchor; we don't need the merged view.
+    // Flagless mode: run portfolio hygiene check. Mostly single-file
+    // semantics (git blame, commit SHAs, etc.), so resolve the anchor -
+    // but the large-lot check inside diffs portfolio content and needs
+    // the merged view, so hand it the whole glob too.
     if (fidelity_csv == null and schwab_csv == null and !schwab_summary) {
         const pf = ctx.resolvePortfolioPath();
         defer pf.deinit(allocator);
-        return hygiene.runHygieneCheck(io, allocator, ctx.environ_map, svc, pf.path, stale_days, verbose, as_of, now_s, color, ctx.globals.refresh_policy, out);
+        var all = ctx.resolvePortfolioPaths() catch null;
+        defer if (all) |*p| p.deinit();
+        const paths: []const []const u8 = if (all) |p| p.paths else &.{pf.path};
+        return hygiene.runHygieneCheck(io, allocator, ctx.environ_map, svc, pf.path, paths, stale_days, verbose, as_of, now_s, color, ctx.globals.refresh_policy, out);
     }
 
     // Reconciliation modes (--fidelity / --schwab / --schwab-summary):

src/commands/audit/hygiene.zig

@@ -729,6 +729,11 @@ pub fn runHygieneCheck(
     env: *const std.process.Environ.Map,
     svc: *zfin.DataService,
     portfolio_path: []const u8,
+    /// Every file in the `portfolio*.srf` glob. Only the large-lot
+    /// check needs this: it diffs portfolio CONTENT and so must see
+    /// the merged view. The rest of the hygiene report is deliberately
+    /// single-file (git blame, commit SHAs) and uses `portfolio_path`.
+    portfolio_paths: []const []const u8,
     stale_days: u32,
     verbose: bool,
     as_of: Date,
@@ -1226,7 +1231,7 @@ pub fn runHygieneCheck(
     // (not in a git repo). Threshold is per-account: an account's
     // `audit_large_lot_threshold` in accounts.srf wins, otherwise the
     // filter's built-in default applies.
-    if (contributions.findUnmatchedLargeLots(io, allocator, env, svc, portfolio_path, &account_map, as_of, color, refresh)) |found| {
+    if (contributions.findUnmatchedLargeLots(io, allocator, env, svc, portfolio_paths, &account_map, as_of, color, refresh)) |found| {
         var found_mut = found;
         defer found_mut.deinit();
 
@@ -2182,7 +2187,7 @@ test "runHygieneCheck: Section 7 flags an un-opted-in symbol's split, not an opt
     var aw: std.Io.Writer.Allocating = .init(allocator);
     defer aw.deinit();
 
-    try runHygieneCheck(io, allocator, &env, &svc, pf_path, 3, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, false, .never, &aw.writer);
+    try runHygieneCheck(io, allocator, &env, &svc, pf_path, &.{pf_path}, 3, false, zfin.Date.fromYmd(2026, 1, 1), 1_767_225_600, false, .never, &aw.writer);
 
     const output = aw.written();
     try std.testing.expect(std.mem.indexOf(u8, output, "Portfolio hygiene") != null);

src/commands/compare.zig

@@ -290,6 +290,16 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     defer pf.deinit(allocator);
     const portfolio_path = pf.path;
 
+    // Attribution diffs portfolio CONTENT, so it needs the whole
+    // `portfolio*.srf` glob rather than the anchor alone - otherwise a
+    // sold lot archived into a sibling file reads as a bare
+    // disappearance and its proceeds can't offset the repurchase.
+    // Resolution failure is non-fatal: the attribution line is
+    // optional, so fall back to the anchor on its own.
+    var attr_pf = ctx.resolvePortfolioPaths() catch null;
+    defer if (attr_pf) |*p| p.deinit();
+    const attr_paths: []const []const u8 = if (attr_pf) |p| p.paths else &.{portfolio_path};
+
     const with_projections = parsed.with_projections;
     const events_enabled = parsed.events_enabled;
     const snapshot_after_live = parsed.after_is_live and parsed.snapshot_after == null;
@@ -471,7 +481,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         // Attribution uses the resolved CommitSpecs so --commit-*
         // overrides + date fallbacks share one classifier. The caller
         // adapts dates to `CommitSpec.date_at_or_before` upstream.
-        const attribution = contributions.computeAttributionSpec(io, allocator, ctx.environ_map, svc, portfolio_path, attr_before, attr_after_opt, as_of, color, ctx.globals.refresh_policy);
+        const attribution = contributions.computeAttributionSpec(io, allocator, ctx.environ_map, svc, attr_paths, attr_before, attr_after_opt, as_of, color, ctx.globals.refresh_policy);
 
         try renderFromParts(out, color, allocator, .{
             .then_date = then_date,
@@ -488,7 +498,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         var now_side = try compare_core.loadSnapshotSide(io, allocator, hist_dir, now_date);
         defer now_side.deinit(allocator);
 
-        const attribution = contributions.computeAttributionSpec(io, allocator, ctx.environ_map, svc, portfolio_path, attr_before, attr_after_opt, as_of, color, ctx.globals.refresh_policy);
+        const attribution = contributions.computeAttributionSpec(io, allocator, ctx.environ_map, svc, attr_paths, attr_before, attr_after_opt, as_of, color, ctx.globals.refresh_policy);
 
         try renderFromParts(out, color, allocator, .{
             .then_date = then_date,

src/git.zig

@@ -207,17 +207,29 @@ pub fn findRepo(io: std.Io, allocator: std.mem.Allocator, env: *const std.proces
     const root = try allocator.dupe(u8, root_raw);
     errdefer allocator.free(root);
 
-    // Relative path from root to the file. If `abs_path` starts with the
-    // repo root (the common case), trim the prefix; otherwise fall back to
-    // just the basename (extremely unusual - repo root disagrees with
-    // path).
+    const rel = try relPathInRoot(allocator, root, abs_path);
+
+    return .{ .root = root, .rel_path = rel };
+}
+
+/// Relative path from `root` to `abs_path`, as git pathspecs want it.
+///
+/// If `abs_path` starts with the repo root (the common case), trim the
+/// prefix; otherwise fall back to just the basename (extremely unusual
+/// - repo root disagrees with path). Caller owns the result.
+pub fn relPathInRoot(allocator: std.mem.Allocator, root: []const u8, abs_path: []const u8) ![]const u8 {
     const rel_raw = if (std.mem.startsWith(u8, abs_path, root) and abs_path.len > root.len)
         std.mem.trimStart(u8, abs_path[root.len..], "/")
     else
         std.fs.path.basename(abs_path);
-    const rel = try allocator.dupe(u8, rel_raw);
+    return allocator.dupe(u8, rel_raw);
+}
 
-    return .{ .root = root, .rel_path = rel };
+/// `relPathInRoot` against an already-discovered repo. Convenience for
+/// callers holding a `RepoInfo` that need rel-paths for the anchor's
+/// sibling files (e.g. every file in a `portfolio*.srf` glob).
+pub fn relPathIn(allocator: std.mem.Allocator, repo: RepoInfo, abs_path: []const u8) ![]const u8 {
+    return relPathInRoot(allocator, repo.root, abs_path);
 }
 
 /// Report the tracked/untracked/modified status of `rel_path` relative to
@@ -396,22 +408,26 @@ pub fn lastCommitTimestampForPath(
     return std.fmt.parseInt(i64, trimmed, 10) catch return null;
 }
 
-/// Return the SHA of the most recent commit that touched `rel_path` at
-/// or before `date_iso` (YYYY-MM-DD, inclusive end-of-day semantics via
-/// `git log --until`).
+/// Return the SHA of the most recent commit that touched any of
+/// `rel_paths` at or before `date_iso` (YYYY-MM-DD, inclusive
+/// end-of-day semantics via `git log --until`).
 ///
-/// Returns null if no commit before `date_iso` touched `rel_path`.
+/// Returns null if no commit before `date_iso` touched any of them.
 /// Caller owns the returned string.
 ///
 /// Used by `zfin contributions --since <DATE>` / `--until <DATE>` to
 /// resolve a date to the last commit that stamped a given snapshot of
-/// the portfolio file.
+/// the portfolio. Takes a slice rather than one path because the
+/// portfolio is a `portfolio*.srf` glob: resolving against only the
+/// first file would pick an older commit whenever the commit in range
+/// touched just a sibling (a sold lot moved into
+/// `portfolio_closed.srf`), silently widening the window.
 pub fn commitAtOrBeforeDate(
     io: std.Io,
     allocator: std.mem.Allocator,
     env: *const std.process.Environ.Map,
     root: []const u8,
-    rel_path: []const u8,
+    rel_paths: []const []const u8,
     date_iso: []const u8,
 ) Error!?[]const u8 {
     // `git log --until=DATE` with a bare YYYY-MM-DD uses the *current
@@ -426,11 +442,22 @@ pub fn commitAtOrBeforeDate(
     const until_arg = try std.fmt.allocPrint(allocator, "--until={s} 23:59:59", .{date_iso});
     defer allocator.free(until_arg);
 
-    const result = runGit(io, allocator, env, &.{
+    // `git log -1 -- p1 p2 ...` already returns the newest commit
+    // touching ANY of the pathspecs, so a multi-file portfolio glob
+    // resolves a date correctly even when the only commit in range
+    // touched just one of the files (e.g. a sold lot moved into
+    // `portfolio_closed.srf`). Argv is built dynamically because the
+    // path count is not known at comptime.
+    var argv: std.ArrayList([]const u8) = .empty;
+    defer argv.deinit(allocator);
+    try argv.appendSlice(allocator, &.{
         "git",     "-C", root,
         "log",     "-1", "--format=%H",
-        until_arg, "--", rel_path,
-    }, .limited(64 * 1024)) catch return error.GitUnavailable;
+        until_arg, "--",
+    });
+    try argv.appendSlice(allocator, rel_paths);
+
+    const result = runGit(io, allocator, env, argv.items, .limited(64 * 1024)) catch return error.GitUnavailable;
     defer allocator.free(result.stdout);
     defer allocator.free(result.stderr);
 
@@ -579,6 +606,7 @@ pub fn resolveCommitRangeSpec(
     arena: std.mem.Allocator,
     env: *const std.process.Environ.Map,
     repo: RepoInfo,
+    rel_paths: []const []const u8,
     before: ?CommitSpec,
     after: ?CommitSpec,
     dirty: bool,
@@ -590,7 +618,7 @@ pub fn resolveCommitRangeSpec(
 
     // Resolve each endpoint independently.
     const before_rev: []const u8 = if (before) |b|
-        try resolveSpec(io, arena, env, repo, b)
+        try resolveSpec(io, arena, env, repo, rel_paths, b)
     else if (dirty)
         "HEAD"
     else
@@ -599,7 +627,7 @@ pub fn resolveCommitRangeSpec(
     const after_rev: ?[]const u8 = if (after) |a|
         (switch (a) {
             .working_copy => null,
-            else => try resolveSpec(io, arena, env, repo, a),
+            else => try resolveSpec(io, arena, env, repo, rel_paths, a),
         })
     else if (dirty)
         null
@@ -612,14 +640,14 @@ pub fn resolveCommitRangeSpec(
 /// Resolve one non-working `CommitSpec` to a string git can consume.
 /// Caller handles the `.working_copy` case separately (it's not a
 /// git ref).
-fn resolveSpec(io: std.Io, arena: std.mem.Allocator, env: *const std.process.Environ.Map, repo: RepoInfo, spec: CommitSpec) Error![]const u8 {
+fn resolveSpec(io: std.Io, arena: std.mem.Allocator, env: *const std.process.Environ.Map, repo: RepoInfo, rel_paths: []const []const u8, spec: CommitSpec) Error![]const u8 {
     return switch (spec) {
         .git_ref => |r| r,
         .date_at_or_before => |d| blk: {
             var buf: [10]u8 = undefined;
             // SAFETY: 10-byte buffer is exactly the size of "YYYY-MM-DD".
             const date_str = std.fmt.bufPrint(&buf, "{f}", .{d}) catch buf[0..];
-            const sha = (try commitAtOrBeforeDate(io, arena, env, repo.root, repo.rel_path, date_str)) orelse
+            const sha = (try commitAtOrBeforeDate(io, arena, env, repo.root, rel_paths, date_str)) orelse
                 return error.NoCommitAtOrBefore;
             break :blk sha;
         },
@@ -639,6 +667,7 @@ pub fn resolveCommitRange(
     arena: std.mem.Allocator,
     env: *const std.process.Environ.Map,
     repo: RepoInfo,
+    rel_paths: []const []const u8,
     since: ?Date,
     until: ?Date,
     dirty: bool,
@@ -646,7 +675,7 @@ pub fn resolveCommitRange(
     std.debug.assert(!(since == null and until != null));
     const before: ?CommitSpec = if (since) |d| .{ .date_at_or_before = d } else null;
     const after: ?CommitSpec = if (until) |d| .{ .date_at_or_before = d } else null;
-    return resolveCommitRangeSpec(io, arena, env, repo, before, after, dirty);
+    return resolveCommitRangeSpec(io, arena, env, repo, rel_paths, before, after, dirty);
 }
 
 // ── Tests ────────────────────────────────────────────────────
@@ -725,7 +754,7 @@ test "commitAtOrBeforeDate returns a SHA for a past date" {
 
     // Any date well after the repo's creation - commitAtOrBeforeDate
     // should find the most recent commit touching build.zig.
-    const sha_opt = commitAtOrBeforeDate(std.testing.io, allocator, &env, info.root, info.rel_path, "2099-01-01") catch return;
+    const sha_opt = commitAtOrBeforeDate(std.testing.io, allocator, &env, info.root, &.{info.rel_path}, "2099-01-01") catch return;
     try std.testing.expect(sha_opt != null);
     const sha = sha_opt.?;
     defer allocator.free(sha);
@@ -744,7 +773,7 @@ test "commitAtOrBeforeDate returns null for date before repo existed" {
     defer allocator.free(info.rel_path);
 
     // Pre-git - before any sensible project history.
-    const sha_opt = commitAtOrBeforeDate(std.testing.io, allocator, &env, info.root, info.rel_path, "1970-01-02") catch return;
+    const sha_opt = commitAtOrBeforeDate(std.testing.io, allocator, &env, info.root, &.{info.rel_path}, "1970-01-02") catch return;
     try std.testing.expect(sha_opt == null);
 }
 
@@ -773,7 +802,7 @@ test "commitAtOrBeforeDate: --until=DATE covers end of day, not current time-of-
 
     // Future-dated cutoff - should always return the tip of history
     // regardless of current wall-clock time.
-    const sha_opt = commitAtOrBeforeDate(std.testing.io, allocator, &env, info.root, info.rel_path, "2099-01-01") catch return;
+    const sha_opt = commitAtOrBeforeDate(std.testing.io, allocator, &env, info.root, &.{info.rel_path}, "2099-01-01") catch return;
     try std.testing.expect(sha_opt != null);
     if (sha_opt) |s| allocator.free(s);
 }
@@ -817,7 +846,7 @@ test "resolveCommitRange: legacy clean -> HEAD~1..HEAD" {
     defer env.deinit();
     const repo: RepoInfo = .{ .root = "/tmp", .rel_path = "portfolio.srf" };
 
-    const range = try resolveCommitRange(std.testing.io, arena_state.allocator(), &env, repo, null, null, false);
+    const range = try resolveCommitRange(std.testing.io, arena_state.allocator(), &env, repo, &.{repo.rel_path}, null, null, false);
     try std.testing.expectEqualStrings("HEAD~1", range.before_rev);
     try std.testing.expectEqualStrings("HEAD", range.after_rev.?);
 }
@@ -829,7 +858,7 @@ test "resolveCommitRange: legacy dirty -> HEAD..working-copy" {
     defer env.deinit();
     const repo: RepoInfo = .{ .root = "/tmp", .rel_path = "portfolio.srf" };
 
-    const range = try resolveCommitRange(std.testing.io, arena_state.allocator(), &env, repo, null, null, true);
+    const range = try resolveCommitRange(std.testing.io, arena_state.allocator(), &env, repo, &.{repo.rel_path}, null, null, true);
     try std.testing.expectEqualStrings("HEAD", range.before_rev);
     try std.testing.expect(range.after_rev == null);
 }
@@ -851,6 +880,7 @@ test "resolveCommitRange: --since resolves to SHA..HEAD for clean tree" {
         arena_state.allocator(),
         &env,
         info,
+        &.{info.rel_path},
         Date.fromYmd(2099, 1, 1),
         null,
         false,
@@ -876,6 +906,7 @@ test "resolveCommitRange: --since with no earlier commit -> NoCommitAtOrBefore"
         arena_state.allocator(),
         &env,
         info,
+        &.{info.rel_path},
         Date.fromYmd(1970, 1, 2),
         null,
         false,

src/portfolio_loader.zig

@@ -270,10 +270,8 @@ pub fn loadPortfolioFromPathsAtRev(
         };
         defer allocator.free(real);
 
-        const rel = if (std.mem.startsWith(u8, real, info.root) and real.len > info.root.len)
-            std.mem.trimStart(u8, real[info.root.len..], "/")
-        else
-            std.fs.path.basename(real);
+        const rel = git.relPathInRoot(allocator, info.root, real) catch return null;
+        defer allocator.free(rel);
 
         const data = git.show(io, allocator, env, info.root, rev, rel) catch |err| switch (err) {
             error.PathMissingInRev => empty_blk: {