diff --git a/docs/guides/track-contributions.md b/docs/guides/track-contributions.md index f929feb..f3f1d8e 100644 --- a/docs/guides/track-contributions.md +++ b/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 diff --git a/docs/reference/cli/contributions.md b/docs/reference/cli/contributions.md index 5019852..1a2ea83 100644 --- a/docs/reference/cli/contributions.md +++ b/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 diff --git a/docs/reference/config/portfolio-srf.md b/docs/reference/config/portfolio-srf.md index 2ff89a3..347b4ab 100644 --- a/docs/reference/config/portfolio-srf.md +++ b/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: diff --git a/docs/reference/config/transaction-log-srf.md b/docs/reference/config/transaction-log-srf.md index 3f85a2a..52318d8 100644 --- a/docs/reference/config/transaction-log-srf.md +++ b/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) diff --git a/src/commands/audit.zig b/src/commands/audit.zig index fb4dedf..360367b 100644 --- a/src/commands/audit.zig +++ b/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): diff --git a/src/commands/audit/hygiene.zig b/src/commands/audit/hygiene.zig index 14df301..b03f9d9 100644 --- a/src/commands/audit/hygiene.zig +++ b/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); diff --git a/src/commands/compare.zig b/src/commands/compare.zig index cb039ea..81811e9 100644 --- a/src/commands/compare.zig +++ b/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, diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index 7a95dd3..2e3bf34 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -46,31 +46,44 @@ //! - `price_only` - same-key, only the `price::` field changed //! - `flagged` - any other edit shape (maturity_date change, etc.) //! -//! ### Intra-account purchase netting (`matchIntraAccountPurchases`) +//! ### Intra-account netting (`matchIntraAccountPurchases`) //! -//! A plain buy made with cash already in the account is not a fresh -//! contribution - the cash was counted when it arrived, and investing -//! it just changes its form. In the diff such a buy is two changes on -//! the SAME account: a `new_stock` / `new_cd` lot appearing, and the -//! account's cash going down (a negative `cash_delta`, or a cash -//! `lot_removed` if the line was fully spent). `matchIntraAccountPurchases` -//! builds a per-account cash-outflow budget from those decreases and -//! draws it down against the account's `new_stock` / `new_cd` lots, -//! recording the funded amount on each Change's `internal_funded`. -//! `attributedValue()` subtracts it, so the funded portion leaves -//! attribution everywhere at once (report, per-account totals, compare, -//! audit large-lot nudge) - no `transaction_log.srf` entry required. +//! Money that was already inside an account is not a fresh +//! contribution - it just changed form. Two shapes of that, both +//! showing up as changes on the SAME account: +//! +//! A plain buy made with cash already in the account: a `new_stock` / +//! `new_cd` lot appearing, and the account's cash going down (a +//! negative `cash_delta`, or a cash `lot_removed` if the line was +//! fully spent). +//! +//! A reallocation - sell A, buy B in the same account: a valued +//! outflow (`lot_removed`, or `drip_negative` for a partial sale) and +//! one or more new lots. Sale proceeds that were spent fund the new +//! lots; proceeds still sitting in cash at window end instead cancel a +//! `cash_contribution`, whose "cash arriving is new money" premise is +//! false for a sale. See `matchIntraAccountPurchases` for the +//! spent/resting split and why it is load-bearing. +//! +//! Either way the funded amount is recorded on the Change's +//! `internal_funded` and `attributedValue()` subtracts it, so the +//! funded portion leaves attribution everywhere at once (report, +//! per-account totals, compare, audit large-lot nudge) - no +//! `transaction_log.srf` entry required. //! //! | Scenario | Kind | Section | In Grand Total | In Attribution | //! |------------------------------------------------|--------------|------------------------|:--------------:|:--------------:| //! | Buy fully funded by same-account cash decrease | `new_stock` | Internal purchases | no | no | //! | Buy partly funded (cash + new money) | `new_stock` | New contributions (residual) + Internal purchases | residual | residual | +//! | Buy funded by a same-account security sale | `new_stock` | Internal purchases | no | no | +//! | Sale proceeds parked in cash on a `cash_is_contribution` account | `cash_contribution` | Internal purchases | no | no | //! //! Runs AFTER the transfer matcher so explicit `transaction_log.srf` //! records win: cash already credited to a `transfer_out` isn't in the //! budget, and a lot already flipped to `transfer_in` is no longer -//! `new_stock` / `new_cd`. Scope is deliberately narrow - only brand-new -//! `new_stock` / `new_cd` lots, same account. `new_drip_lot`, +//! `new_stock` / `new_cd`. Scope on the destination side is deliberately +//! narrow - only brand-new `new_stock` / `new_cd` lots and +//! `cash_contribution`, same account. `new_drip_lot`, //! `rollup_delta` / `drip_confirmed`, and `partial_transfer_in` //! residuals are left untouched (see `matchIntraAccountPurchases`). //! @@ -232,6 +245,8 @@ const framework = @import("framework.zig"); const TimeRange = @import("TimeRange.zig"); const analysis = @import("../analytics/analysis.zig"); const transaction_log = @import("../models/transaction_log.zig"); +const portfolio_loader = @import("../portfolio_loader.zig"); +const test_git = @import("../testutil/git.zig"); const Money = @import("../Money.zig"); const Date = zfin.Date; const Lot = zfin.Lot; @@ -368,11 +383,17 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { const before = parsed.before; const after = parsed.after; - const pf = ctx.resolvePortfolioPath(); - defer pf.deinit(allocator); - const portfolio_path = pf.path; + var pf = ctx.resolvePortfolioPaths() catch { + cli.stderrPrint(io, "Error: could not resolve portfolio file(s).\n"); + return; + }; + defer pf.deinit(); + if (pf.paths.len == 0) { + cli.stderrPrint(io, "Error: No portfolio file found\n"); + return; + } - return runImpl(io, allocator, ctx.environ_map, svc, portfolio_path, before, after, as_of, color, ctx.globals.refresh_policy, out); + return runImpl(io, allocator, ctx.environ_map, svc, pf.paths, before, after, as_of, color, ctx.globals.refresh_policy, out); } fn runImpl( @@ -380,7 +401,7 @@ fn runImpl( allocator: std.mem.Allocator, env: *const std.process.Environ.Map, svc: *zfin.DataService, - portfolio_path: []const u8, + paths: []const []const u8, before: ?git.CommitSpec, after: ?git.CommitSpec, as_of: Date, @@ -405,7 +426,7 @@ fn runImpl( return; } - var ctx = prepareReport(io, allocator, arena, env, svc, portfolio_path, before, after, as_of, color, refresh, .verbose) catch return; + var ctx = prepareReport(io, allocator, arena, env, svc, paths, before, after, as_of, color, refresh, .verbose) catch return; defer ctx.deinit(); try printReport(out, &ctx.report, ctx.endpoints.label, color); @@ -416,33 +437,43 @@ fn runImpl( /// both need from the git-backed diff. /// /// Owned fields split across two allocators: -/// - `before_pf` / `after_pf` use the base allocator (their own -/// `deinit` frees internals). -/// - `endpoints`, `report`, and the snapshot blobs live in the -/// supplied arena; the arena's own `deinit` cleans them up. +/// - `before_pf` / `after_pf` are merged multi-file loads on the base +/// allocator (their own `deinit` frees lots and file bytes). +/// - `endpoints` and `report` live in the supplied arena; the arena's +/// own `deinit` cleans them up. /// `deinit` releases only the base-allocator-owned pieces. const ReportContext = struct { endpoints: Endpoints, - before_pf: zfin.Portfolio, - after_pf: zfin.Portfolio, + before_pf: portfolio_loader.LoadedPortfolio, + after_pf: portfolio_loader.LoadedPortfolio, report: Report, + allocator: std.mem.Allocator, fn deinit(self: *ReportContext) void { - self.before_pf.deinit(); - self.after_pf.deinit(); + self.before_pf.deinit(self.allocator); + self.after_pf.deinit(self.allocator); } }; const PrepareError = error{PrepareFailed}; -/// Run the common pipeline - resolve endpoints, read both blobs, -/// parse both portfolios, fetch prices, build the report. +/// Run the common pipeline - resolve endpoints, load the merged +/// portfolio at both revisions, fetch prices, build the report. /// /// Shared between `run` (prints the report) and /// `computeAttributionImpl` (aggregates totals). Centralizes the git /// plumbing and the price-loading step; callers own their output /// decisions. /// +/// `paths` is the whole `portfolio*.srf` glob, and every file is read +/// at both revisions and merged - the same union the live loader and +/// the TUI see. Reading only the first file made a sold lot moved into +/// a sibling `portfolio_closed.srf` look like a bare disappearance, +/// and hid the `close_price` that records what the sale actually +/// realized. `paths[0]` remains the anchor for sibling-file derivation +/// (`accounts.srf`, `transaction_log.srf`), matching +/// `LoadedPortfolio`'s documented convention. +/// /// Stderr output is gated by `verbosity`: `.verbose` is the `run` /// path (user sees why things failed); `.silent` is the attribution /// path (failure just means "no attribution line", don't nag). @@ -452,7 +483,7 @@ fn prepareReport( arena: std.mem.Allocator, env: *const std.process.Environ.Map, svc: *zfin.DataService, - portfolio_path: []const u8, + paths: []const []const u8, before_spec: ?git.CommitSpec, after_spec: ?git.CommitSpec, as_of: Date, @@ -460,6 +491,11 @@ fn prepareReport( refresh: framework.RefreshPolicy, verbosity: Verbosity, ) PrepareError!ReportContext { + if (paths.len == 0) { + if (verbosity == .verbose) cli.stderrPrint(io, "Error: No portfolio file found\n"); + return error.PrepareFailed; + } + const portfolio_path = paths[0]; const repo = git.findRepo(io, arena, env, portfolio_path) catch |err| { if (verbosity == .verbose) { switch (err) { @@ -471,65 +507,63 @@ fn prepareReport( return error.PrepareFailed; }; - const status = git.pathStatus(io, arena, env, repo.root, repo.rel_path) catch { - if (verbosity == .verbose) cli.stderrPrint(io, "Error: could not determine git status of portfolio.srf.\n"); - return error.PrepareFailed; - }; - if (status == .untracked) { - if (verbosity == .verbose) cli.stderrPrint(io, "Error: portfolio.srf is not tracked in git. Add and commit it first.\n"); - return error.PrepareFailed; - } - const dirty = status == .modified; - - const endpoints = resolveEndpoints(io, arena, env, repo, before_spec, after_spec, dirty, verbosity) catch return error.PrepareFailed; - - // Pull both sides: before is always from git; after is either - // from git (at some revision) or from the working copy. - const before = git.show(io, arena, env, repo.root, endpoints.range.before_rev, repo.rel_path) catch |err| { - if (verbosity == .verbose) { - var buf: [256]u8 = undefined; - const msg = std.fmt.bufPrint(&buf, "Error reading {s}:portfolio.srf from git: {s}\n", .{ endpoints.range.before_rev, @errorName(err) }) catch "Error reading before-side portfolio.\n"; - cli.stderrPrint(io, msg); - } - return error.PrepareFailed; - }; - - const after = if (endpoints.range.after_rev) |rev| - git.show(io, arena, env, repo.root, rev, repo.rel_path) catch |err| { + // Dirty/untracked status covers EVERY portfolio file, not just the + // anchor: editing a sibling is just as much a working-copy change, + // and treating the tree as clean would diff HEAD~1..HEAD and miss + // the edit entirely. + var rel_paths: std.ArrayList([]const u8) = .empty; + var dirty = false; + for (paths) |abs| { + const rel = git.relPathIn(arena, repo, abs) catch return error.PrepareFailed; + rel_paths.append(arena, rel) catch return error.PrepareFailed; + const status = git.pathStatus(io, arena, env, repo.root, rel) catch { + if (verbosity == .verbose) cli.stderrPrint(io, "Error: could not determine git status of portfolio.srf.\n"); + return error.PrepareFailed; + }; + if (status == .untracked) { if (verbosity == .verbose) { - var buf: [256]u8 = undefined; - const msg = std.fmt.bufPrint(&buf, "Error reading {s}:portfolio.srf from git: {s}\n", .{ rev, @errorName(err) }) catch "Error reading after-side portfolio.\n"; + var buf: [512]u8 = undefined; + const msg = std.fmt.bufPrint(&buf, "Error: {s} is not tracked in git. Add and commit it first.\n", .{rel}) catch "Error: portfolio file is not tracked in git. Add and commit it first.\n"; cli.stderrPrint(io, msg); } return error.PrepareFailed; } + if (status == .modified) dirty = true; + } + + const endpoints = resolveEndpoints(io, arena, env, repo, rel_paths.items, before_spec, after_spec, dirty, verbosity) catch return error.PrepareFailed; + + // Load the merged view at both endpoints: before is always from + // git; after is either from git (at some revision) or from the + // working copy. Files absent at a revision are skipped, so a + // portfolio that gained a sibling file mid-history still diffs. + var before_pf = portfolio_loader.loadPortfolioFromPathsAtRev(io, allocator, env, paths, endpoints.range.before_rev, as_of) orelse { + if (verbosity == .verbose) cli.stderrPrint(io, "Error reading before-side portfolio from git.\n"); + return error.PrepareFailed; + }; + errdefer before_pf.deinit(allocator); + + var after_pf = if (endpoints.range.after_rev) |rev| + portfolio_loader.loadPortfolioFromPathsAtRev(io, allocator, env, paths, rev, as_of) orelse { + if (verbosity == .verbose) cli.stderrPrint(io, "Error reading after-side portfolio from git.\n"); + return error.PrepareFailed; + } else - std.Io.Dir.cwd().readFileAlloc(io, portfolio_path, arena, .limited(10 * 1024 * 1024)) catch { + portfolio_loader.loadPortfolioFromPaths(io, allocator, paths, as_of) orelse { if (verbosity == .verbose) cli.stderrPrint(io, "Error reading working-copy portfolio file.\n"); return error.PrepareFailed; }; - - var before_pf = zfin.cache.deserializePortfolio(allocator, before) catch { - if (verbosity == .verbose) cli.stderrPrint(io, "Error parsing before-snapshot portfolio.\n"); - return error.PrepareFailed; - }; - errdefer before_pf.deinit(); - - var after_pf = zfin.cache.deserializePortfolio(allocator, after) catch { - if (verbosity == .verbose) cli.stderrPrint(io, "Error parsing after-snapshot portfolio.\n"); - return error.PrepareFailed; - }; - errdefer after_pf.deinit(); + errdefer after_pf.deinit(allocator); // Fetch current prices (cache-hit preferred) for DRIP/share-delta valuation. var prices = std.StringHashMap(f64).init(arena); var sym_set = std.StringHashMap(void).init(arena); - for (before_pf.lots) |l| { + for (before_pf.portfolio.lots) |l| { if (l.security_type == .stock and !(l.price != null and l.ticker == null)) { sym_set.put(l.priceSymbol(), {}) catch return error.PrepareFailed; } } - for (after_pf.lots) |l| { + for (after_pf.portfolio.lots) |l| { if (l.security_type == .stock and !(l.price != null and l.ticker == null)) { sym_set.put(l.priceSymbol(), {}) catch return error.PrepareFailed; } @@ -597,8 +631,8 @@ fn prepareReport( const report = computeReport( arena, - before_pf.lots, - after_pf.lots, + before_pf.portfolio.lots, + after_pf.portfolio.lots, &prices, as_of, .{ @@ -614,6 +648,7 @@ fn prepareReport( .endpoints = endpoints, .before_pf = before_pf, .after_pf = after_pf, + .allocator = allocator, .report = report, }; } @@ -641,12 +676,13 @@ fn resolveEndpoints( arena: std.mem.Allocator, env: *const std.process.Environ.Map, repo: git.RepoInfo, + rel_paths: []const []const u8, before: ?git.CommitSpec, after: ?git.CommitSpec, dirty: bool, verbosity: Verbosity, ) !Endpoints { - const range = git.resolveCommitRangeSpec(io, arena, env, repo, before, after, dirty) catch |err| { + const range = git.resolveCommitRangeSpec(io, arena, env, repo, rel_paths, before, after, dirty) catch |err| { if (verbosity == .verbose) { switch (err) { error.NoCommitAtOrBefore => { @@ -915,7 +951,7 @@ pub fn computeAttributionSpec( allocator: std.mem.Allocator, env: *const std.process.Environ.Map, svc: *zfin.DataService, - portfolio_path: []const u8, + paths: []const []const u8, before: ?git.CommitSpec, after: ?git.CommitSpec, as_of: Date, @@ -928,7 +964,7 @@ pub fn computeAttributionSpec( defer arena_state.deinit(); const arena = arena_state.allocator(); - var ctx = prepareReport(io, allocator, arena, env, svc, portfolio_path, before, after, as_of, color, refresh, .silent) catch return null; + var ctx = prepareReport(io, allocator, arena, env, svc, paths, before, after, as_of, color, refresh, .silent) catch return null; defer ctx.deinit(); return summarizeAttribution(ctx); @@ -1008,7 +1044,7 @@ pub fn findUnmatchedLargeLots( allocator: std.mem.Allocator, env: *const std.process.Environ.Map, svc: *zfin.DataService, - portfolio_path: []const u8, + paths: []const []const u8, account_map: ?*const analysis.AccountMap, as_of: Date, color: bool, @@ -1026,7 +1062,7 @@ pub fn findUnmatchedLargeLots( // // Separate allocator here so we can tear the whole thing down // via `arena_state.deinit` once we've copied out the descriptors. - var ctx = prepareReport(io, allocator, arena, env, svc, portfolio_path, null, null, as_of, color, refresh, .silent) catch { + var ctx = prepareReport(io, allocator, arena, env, svc, paths, null, null, as_of, color, refresh, .silent) catch { arena_state.deinit(); return null; }; @@ -1170,6 +1206,25 @@ const ChangeKind = enum { cd_matured, // lot disappeared: CD with maturity_date <= today cd_removed_early, // lot disappeared: CD with maturity_date > today lot_removed, // lot disappeared: stock/cash/option + /// A lot that gained `close_date` (and normally `close_price`) in + /// place: the position was sold, but the record was kept rather + /// than deleted - either edited where it sat, or archived into a + /// sibling `portfolio_closed.srf` that the same glob picks up. + /// + /// Needs its own kind because the strict lot key deliberately + /// excludes `close_date`, so a close leaves the key and the share + /// count untouched. Without this the change fell through the + /// same-shares branch (which only inspects `price` and + /// `maturity_date`) and emitted nothing at all, making the sale + /// invisible to attribution and letting the repurchase read as a + /// fresh contribution. + /// + /// Valued at `close_price` - the only figure that records what the + /// sale actually realized, and strictly better than the current-price + /// proxy `lot_removed` has to settle for. Contributes $0 to + /// attribution and acts as a funding source, exactly like + /// `lot_removed`. + position_closed, /// Strict lot key broke (open_date / open_price / symbol rewritten) /// but the same (security_type, priceSymbol, account) reappears on /// the other side with approximately the same share total. @@ -1222,7 +1277,10 @@ const Change = struct { delta_shares: f64 = 0, /// Price used to value delta_shares (open_price, current price, or manual price). unit_value: f64 = 0, - /// For cd_matured / cd_removed_early / lot_removed: the before-lot's shares. + /// For cd_matured / cd_removed_early / lot_removed: the dollar + /// value that left, i.e. `before_shares * unit_value`. For a cash + /// lot that is shares 1:1; for a CD it is the face value; for a + /// stock it is the sale proceeds at the best available price. face_value: f64 = 0, /// For cd_matured / cd_removed_early: maturity_date. maturity_date: ?Date = null, @@ -1256,16 +1314,24 @@ const Change = struct { transfer_date: ?Date = null, // ── Intra-account purchase netting (see `matchIntraAccountPurchases`) ──── - /// Portion of this Change's `value()` funded by a decrease in the - /// SAME account's cash during the window - i.e. a plain buy of a - /// new lot using cash that was already sitting in the account - /// (cash -> security). Set only for `new_stock` / `new_cd`. The - /// matched cash leaves as a negative `cash_delta` (or a removed - /// cash lot) on the same account, so funding the purchase with it - /// is internal movement, not a fresh contribution. `attributedValue()` - /// subtracts it; a fully-funded buy nets to $0 and drops out of - /// "New contributions", the per-account totals, the compare - /// attribution line, and the audit large-lot nudge. Zero otherwise. + /// Portion of this Change's `value()` funded by money that was + /// already inside the SAME account - not a fresh contribution. + /// `attributedValue()` subtracts it, so a fully-funded change nets + /// to $0 and drops out of "New contributions", the per-account + /// totals, the compare attribution line, and the audit large-lot + /// nudge. Zero otherwise. Two funding sources, both same-account: + /// + /// - **cash that visibly left** (a negative `cash_delta` or a + /// removed cash lot) funding a `new_stock` / `new_cd` buy; + /// - **proceeds of a security sale** - a valued `lot_removed` / + /// `drip_negative`. Proceeds that were spent fund `new_stock` / + /// `new_cd`; proceeds still sitting in cash at window end + /// instead cancel a `cash_contribution` on that account, which + /// would otherwise book the sale as new money (see + /// `matchIntraAccountPurchases` for the spent/resting split). + /// + /// So the kinds that can carry this are `new_stock`, `new_cd`, and + /// `cash_contribution`. /// /// Distinct from `transfer_attributed` (which tracks cross-account /// movement declared in `transaction_log.srf`); the two are @@ -1285,17 +1351,28 @@ const Change = struct { /// matcher may have credited some of the value to a cash- /// destination transfer record (see `matchCashDestination`); /// `transfer_attributed` tracks how much. For `new_stock` / - /// `new_cd`, `internal_funded` tracks how much of the purchase was - /// funded by a same-account cash decrease (see + /// `new_cd` / `cash_contribution`, `internal_funded` tracks how + /// much was funded from inside the same account - a cash decrease + /// or the proceeds of a security sale (see /// `matchIntraAccountPurchases`). The residual is /// `value() - transfer_attributed - internal_funded`, which is /// what shows up in "New contributions / purchases" and the audit /// large-lot filter. Everyone else sees `value()` unchanged. + /// + /// Outflows are pinned to 0 rather than falling through to + /// `value()`. They are a *funding source*, never a negative + /// contribution: the money did not leave the portfolio, it changed + /// form. Letting a valued `lot_removed` reach the `else` arm below + /// would subtract the sale from contributions and break the + /// `delta = contributions + gains` identity in the opposite + /// direction from the bug this valuation was added to fix. pub fn attributedValue(self: Change) f64 { return switch (self.kind) { .transfer_in, .transfer_out, .unmatched_transfer => 0, + .lot_removed, .position_closed, .drip_negative, .cd_matured, .cd_removed_early => 0, .partial_transfer_in => self.value() - self.transfer_attributed, - .new_cash, .cash_contribution, .cash_delta => self.value() - self.transfer_attributed, + .new_cash, .cash_delta => self.value() - self.transfer_attributed, + .cash_contribution => self.value() - self.transfer_attributed - self.internal_funded, .new_stock, .new_cd => self.value() - self.transfer_attributed - self.internal_funded, else => self.value(), }; @@ -1374,6 +1451,61 @@ fn secondaryKey(allocator: std.mem.Allocator, lot: Lot) ![]u8 { }); } +/// Dollars-per-share for a lot that is LEAVING the portfolio (a whole +/// lot removed in pass 2, or a position closed in place). Multiplied +/// by the share count it gives the sale proceeds, which is what makes +/// a sale usable as a funding source for a same-account purchase. +/// +/// A sale's true proceeds are only knowable from `close_price` (see +/// `closeUnitValue`); this is the fallback for a lot that simply +/// vanished from the file with no closing record. Current cached +/// price is the best available proxy - the diff windows are short +/// (typically one commit), so a recent sale prices close to the mark. +/// Manual `price::` then `open_price` follow; `open_price` is a cost +/// basis and understates a long-held winner, so it really is a last +/// resort. +/// +/// `prices.get` returns the raw retail-class API price -> ratio +/// applies. `lot.price` and `lot.open_price` are already in the lot's +/// own share-class terms (preadjusted) -> ratio must NOT be applied. +/// See the "Pricing model" doc-block in models/portfolio.zig. +fn outflowUnitValue(lot: Lot, prices: *const std.StringHashMap(f64)) f64 { + return switch (lot.security_type) { + .stock => blk: { + if (prices.get(lot.priceSymbol())) |p| break :blk lot.effectivePrice(p, false); + if (lot.price) |p| break :blk lot.effectivePrice(p, true); + break :blk lot.effectivePrice(lot.open_price, true); + }, + // Shares ARE dollars for cash lots. + .cash => 1.0, + // Face value per share. + .cd => lot.open_price, + .option => lot.open_price * lot.multiplier, + else => lot.open_price, + }; +} + +/// Dollars-per-share realized when a lot was closed in place. +/// +/// `close_price` is authoritative - it is what the sale actually got, +/// recorded at the time by whoever closed the lot - so it wins +/// outright. Cost basis would understate a long-held winner badly +/// (on the trade that motivated this, by $48,963.65 on $467k), which +/// is why the fallback defers to `outflowUnitValue`'s current-price +/// proxy rather than reaching for `open_price`. +fn closeUnitValue(lot: Lot, prices: *const std.StringHashMap(f64)) f64 { + if (lot.close_price) |cp| { + return switch (lot.security_type) { + // close_price is in the lot's own share-class terms + // (preadjusted), so the ratio must NOT be applied again. + .stock => lot.effectivePrice(cp, true), + .option => cp * lot.multiplier, + else => cp, + }; + } + return outflowUnitValue(lot, prices); +} + /// Tolerance for "did the share total stay the same" check when /// deciding whether to emit a rollup_delta for any residual share /// difference alongside a `lot_edited` classification. Anything within @@ -1735,8 +1867,24 @@ fn computeReport( .unit_value = unit_value, }); } else { - // Same shares. Check for price:: field or other metadata changes. + // Same shares. A close transition wins over any other + // metadata comparison: a lot that was both closed and + // repriced in the same window is a sale, not a price + // edit. Checked first for exactly that reason. const before_lot = before_agg.lot; + if (before_lot.close_date == null and lot.close_date != null) { + const unit_value = closeUnitValue(lot, prices); + try changes.append(allocator, .{ + .kind = .position_closed, + .symbol = sym, + .account = acct, + .security_type = lot.security_type, + .unit_value = unit_value, + .face_value = after_agg.shares * unit_value, + .delta_shares = -after_agg.shares, + }); + continue; + } const a_price = lot.price; const b_price = before_lot.price; if ((a_price != null) != (b_price != null) or @@ -1833,12 +1981,21 @@ fn computeReport( } } + // Value the outflow. `unit_value` makes `value()` the (negative) + // dollar proceeds, which is what lets a security sale fund a + // same-account purchase in `matchIntraAccountPurchases`. Before + // this was populated, `value()` was 0 for every removal and a + // sale funded nothing - so an intra-account reallocation read as + // a full external contribution. + const unit_value = outflowUnitValue(lot, prices); + try changes.append(allocator, .{ .kind = kind, .symbol = sym, .account = acct, .security_type = lot.security_type, - .face_value = before_agg.shares * (if (lot.security_type == .cd) lot.open_price else 1.0), + .unit_value = unit_value, + .face_value = before_agg.shares * unit_value, .maturity_date = lot.maturity_date, .delta_shares = -before_agg.shares, }); @@ -2360,7 +2517,7 @@ fn tryMatchFromSide( rec: transaction_log.TransferRecord, ) void { for (changes.items) |*c| switch (c.kind) { - .cash_delta, .lot_removed => { + .cash_delta, .lot_removed, .position_closed => { if (!std.mem.eql(u8, c.account, rec.from)) continue; const abs_val = @abs(c.value()); if (abs_val < rec.amount - transfer_amount_tolerance) continue; @@ -2457,7 +2614,7 @@ fn matchInKindTransfer( if (!std.mem.eql(u8, c.account, rec.from)) continue; if (!std.mem.eql(u8, c.symbol, dl.symbol)) continue; switch (c.kind) { - .lot_removed, .drip_negative => {}, + .lot_removed, .position_closed, .drip_negative => {}, else => continue, } src_idx = i; @@ -2512,24 +2669,52 @@ fn matchInKindTransfer( // ── Intra-account purchase netting ─────────────────────────── -/// Net same-account cash decreases against new purchase lots. +/// Net same-account internal money movement against the changes that +/// would otherwise read as fresh contributions. /// -/// A plain buy made with cash already in the account shows up in the -/// diff as two changes on the SAME account: a `new_stock` / `new_cd` -/// lot appearing, and the account's cash going down (a negative -/// `cash_delta`, or a `lot_removed` if the cash line was fully -/// consumed). No new money entered the portfolio - the cash was -/// already counted when it arrived - so the purchase shouldn't read -/// as a fresh contribution. +/// Two things can fund a purchase without any new money entering the +/// portfolio, and both show up as changes on the SAME account: /// -/// This pass builds a per-account "cash outflow" budget from those -/// decreases and draws it down against the account's `new_stock` / -/// `new_cd` changes, accumulating the funded amount onto each Change's -/// `internal_funded`. `attributedValue()` subtracts it, so a fully- -/// funded buy nets to $0 (drops out of "New contributions", per-account -/// totals, the compare attribution line, and the audit large-lot -/// nudge) while a partially-funded buy keeps the unfunded residual as -/// real new money. +/// 1. **Cash already in the account.** A plain buy shows a +/// `new_stock` / `new_cd` lot appearing and the account's cash +/// going down (a negative `cash_delta`, or a `lot_removed` if the +/// cash line was fully consumed). +/// 2. **Proceeds of a security sale.** A reallocation - sell A, buy B +/// in the same account - shows a valued outflow (`lot_removed`, or +/// `drip_negative` for a partial sale) and one or more new lots. +/// +/// Case 2 was previously invisible: the budget only accepted cash, so +/// a $467k FAGIX -> SPHY/FDVV swap booked the full $467k as a +/// contribution, and because `compare` derives +/// `gains = liquid.delta - contributions` the phantom appeared again as +/// an equal-and-opposite loss. +/// +/// ## Where the proceeds went decides what they can fund +/// +/// Sale proceeds cannot fund a purchase if they are demonstrably still +/// sitting in cash at the end of the window. Splitting on the +/// account's net cash movement: +/// +/// ``` +/// proceeds_resting = min(sale_proceeds, max(0, cash_change)) +/// proceeds_spent = sale_proceeds - proceeds_resting +/// ``` +/// +/// - `proceeds_spent` funds `new_stock` / `new_cd`. +/// - `proceeds_resting` instead cancels a `cash_contribution` on the +/// account. That kind only exists on accounts flagged +/// `cash_is_contribution::true`, where a positive cash delta is +/// assumed to be new money - an assumption that is wrong for sale +/// proceeds, and that the flag cannot distinguish on its own. +/// +/// Without the split, a window holding both a sale whose proceeds +/// stayed in cash and a genuine deposit that was invested would let +/// the sale swallow the deposit. +/// +/// Cash decreases are drawn *after* sale proceeds, so the two cannot +/// double-count the same dollars: proceeds that passed through cash on +/// their way into a security are already absorbed, and the drawdown +/// caps each lot at its remaining unattributed value. /// /// Runs after `matchTransfers`, so: /// - Cash already reclassified to `transfer_out` (an outflow to a @@ -2539,8 +2724,9 @@ fn matchInKindTransfer( /// are no longer `new_stock` / `new_cd`, so an explicit transfer /// record always takes priority over this automatic netting. /// -/// Scope is deliberately narrow - only brand-new `new_stock` / `new_cd` -/// lots. `new_drip_lot` (a reinvested dividend, not a cash buy), +/// Scope on the destination side stays narrow - only brand-new +/// `new_stock` / `new_cd` lots and `cash_contribution`. +/// `new_drip_lot` (a reinvested dividend, not a cash buy), /// `rollup_delta` / `drip_confirmed` (share adds to an existing lot), /// and `partial_transfer_in` residuals are left untouched. Same-account /// only; cross-account movement stays `transaction_log.srf`'s job. @@ -2553,11 +2739,12 @@ fn matchInKindTransfer( /// funded portion on each. Returns whatever the account's lots could not /// absorb. /// -/// Shared by the two things that can fund a purchase without it being new -/// money: cash that visibly left the same account, and a declared transfer -/// whose cash was spent before it could be observed. The drawdown is identical; -/// only the meaning of a leftover differs, which is why the callers handle the -/// return value differently rather than this function deciding. +/// Shared by the three things that can fund a purchase without it being new +/// money: cash that visibly left the same account, the proceeds of a +/// same-account security sale, and a declared transfer whose cash was spent +/// before it could be observed. The drawdown is identical; only the meaning of +/// a leftover differs, which is why the callers handle the return value +/// differently rather than this function deciding. fn drawDownAgainstNewLots(changes: *std.ArrayList(Change), account: []const u8, budget: f64) f64 { var remaining = budget; for (changes.items) |*c| { @@ -2622,42 +2809,105 @@ const FundingShortfall = struct { rec: transaction_log.TransferRecord, }; +/// Per-account internal-funding flows, gathered in one pass over the +/// changes so the spent/resting split can be computed before any +/// drawdown happens. +const AccountFlows = struct { + /// Dollars realized by selling securities in this account. + sale_proceeds: f64 = 0, + /// Signed net movement of the account's cash pool. Positive means + /// the account is holding more cash at the end of the window. + cash_change: f64 = 0, +}; + fn matchIntraAccountPurchases( allocator: std.mem.Allocator, changes: *std.ArrayList(Change), ) !void { - // Per-account cash outflow: magnitude of net cash that left the - // account's cash pool this window. Negative cash_delta is a partial - // spend; a removed cash lot is a fully-drained line (its dollar - // amount lives in `face_value`, since `value()` is 0 for removals). - var outflow: std.StringHashMap(f64) = .init(allocator); - defer outflow.deinit(); + var flows: std.StringHashMap(AccountFlows) = .init(allocator); + defer flows.deinit(); + for (changes.items) |c| { + const gop = try flows.getOrPut(c.account); + if (!gop.found_existing) gop.value_ptr.* = .{}; + const f = gop.value_ptr; switch (c.kind) { - .cash_delta => { - const v = c.value(); - if (v < 0) { - const gop = try outflow.getOrPut(c.account); - if (!gop.found_existing) gop.value_ptr.* = 0; - gop.value_ptr.* += -v; - } - }, + // Cash pool moved. `cash_contribution` is a positive + // cash_delta on a flagged account, so it counts here too - + // it is still cash arriving. + .cash_delta, .cash_contribution, .new_cash => f.cash_change += c.value(), + // A removed lot is a fully-drained line: its dollar amount + // lives in `face_value` because `value()` is negative for + // removals. Cash lines drain the pool; securities realize + // proceeds. .lot_removed => if (c.security_type == .cash) { - const gop = try outflow.getOrPut(c.account); - if (!gop.found_existing) gop.value_ptr.* = 0; - gop.value_ptr.* += c.face_value; + f.cash_change -= c.face_value; + } else { + f.sale_proceeds += c.face_value; }, + // Closed in place, valued at close_price. Cash lines that + // get closed rather than deleted drain the pool instead. + .position_closed => if (c.security_type == .cash) { + f.cash_change -= c.face_value; + } else { + f.sale_proceeds += c.face_value; + }, + // Partial sale of an existing stock lot: shares went down + // on an unchanged key, so `value()` is the negative + // proceeds. + .drip_negative => f.sale_proceeds += -c.value(), else => {}, } } - if (outflow.count() == 0) return; - // Draw each account's outflow down against its new purchase lots. A - // leftover here is unremarkable - cash can leave an account for reasons - // other than buying something - so it is simply discarded. - var oit = outflow.iterator(); - while (oit.next()) |e| { - _ = drawDownAgainstNewLots(changes, e.key_ptr.*, e.value_ptr.*); + var it = flows.iterator(); + while (it.next()) |e| { + const account = e.key_ptr.*; + const f = e.value_ptr.*; + + // Proceeds still sitting in cash cannot have funded a purchase. + // Split on the account's net cash movement so a sale whose + // proceeds stayed put can't swallow a genuine deposit that was + // invested in the same window. + const resting = @min(f.sale_proceeds, @max(0, f.cash_change)); + const spent = f.sale_proceeds - resting; + + // Spent proceeds fund new security lots. A leftover is + // unremarkable (proceeds can go anywhere) so it is discarded. + if (spent > 0) _ = drawDownAgainstNewLots(changes, account, spent); + + // Resting proceeds cancel a `cash_contribution`, which would + // otherwise book the sale as new money on an account flagged + // `cash_is_contribution::true`. + if (resting > 0) drawDownAgainstCashContribution(changes, account, resting); + + // Finally, cash that visibly left funds whatever new lot value + // is still unattributed. Drawn last so proceeds that passed + // through cash on their way into a security aren't counted + // twice. + if (f.cash_change < 0) _ = drawDownAgainstNewLots(changes, account, -f.cash_change); + } +} + +/// Draw `budget` down against `cash_contribution` changes in `account`. +/// +/// Mirrors `drawDownAgainstNewLots` but targets the opt-in +/// cash-is-contribution kind, whose whole premise - "cash arriving here +/// is new money" - is false for the proceeds of a security sale. Any +/// leftover is discarded: the flag exists precisely because cash can +/// arrive from outside, so an unabsorbed remainder is a real +/// contribution. +fn drawDownAgainstCashContribution(changes: *std.ArrayList(Change), account: []const u8, budget: f64) void { + var remaining = budget; + for (changes.items) |*c| { + if (remaining <= 0) break; + if (c.kind != .cash_contribution) continue; + if (!std.mem.eql(u8, c.account, account)) continue; + const unattributed = c.attributedValue(); + if (unattributed <= 0) continue; + const draw = @min(unattributed, remaining); + c.internal_funded += draw; + remaining -= draw; } } @@ -2799,24 +3049,34 @@ fn printReport(out: *std.Io.Writer, report: *const Report, label: []const u8, co if (!any) try printNone(out, color, mut_color); try out.writeAll("\n"); - // ── Section: Internal purchases (cash-funded - not counted) ── + // ── Section: Internal purchases (internally funded - not counted) ── // - // new_stock / new_cd lots whose value was funded (wholly or in - // part) by a decrease in the SAME account's cash this window - a - // plain buy of existing cash, reclassified by - // `matchIntraAccountPurchases`. The funded portion is internal - // movement, not a fresh contribution, so it's shown here (muted) - // rather than counted in "New contributions". A partially-funded - // lot also appears in "New contributions" on its unfunded residual. + // Two halves of the same story, kept together so the money can be + // followed: + // + // - Sales that released funds: `position_closed` (closed in + // place, valued at close_price) and non-cash `lot_removed` + // (the record was deleted outright). Collapsed by + // account+symbol, because closing a DRIP-fed position can + // retire hundreds of lots and one line per lot buries the + // report - the real trade that prompted this retired 219. + // - `new_stock` / `new_cd` lots, and `cash_contribution`, whose + // value was funded (wholly or in part) by that money or by a + // decrease in the SAME account's cash, as attributed by + // `matchIntraAccountPurchases`. The funded portion is internal + // movement, not a fresh contribution, so it shows here (muted) + // rather than counted in "New contributions". A partially-funded + // lot also appears there on its unfunded residual. var any_internal = false; for (report.changes) |c| { - if (c.internal_funded > 0) { + if (c.internal_funded > 0 or isSaleKind(c)) { any_internal = true; break; } } if (any_internal) { - try printSection(out, "Internal purchases (cash -> securities, not counted)", color, h_color); + try printSection(out, "Internal purchases (cash / sale proceeds -> securities, not counted)", color, h_color); + try printCollapsedSales(out, report, color, mut_color); for (report.changes) |c| { if (c.internal_funded <= 0) continue; try printInternalPurchaseLine(out, c, color, mut_color); @@ -2902,13 +3162,22 @@ fn printReport(out: *std.Io.Writer, report: *const Report, label: []const u8, co // ── Section: Flagged ── var any_flag = false; for (report.changes) |c| switch (c.kind) { - .flagged, .lot_removed, .drip_negative, .unmatched_transfer => any_flag = true, + .flagged, .unmatched_transfer => any_flag = true, + .lot_removed, .drip_negative => if (!isSaleKind(c)) { + any_flag = true; + }, else => {}, }; if (any_flag) { try printSection(out, "Flagged for review", color, h_color); for (report.changes) |c| switch (c.kind) { - .flagged, .lot_removed, .drip_negative => { + .flagged => { + try printFlaggedLine(out, c, color, warn_color); + }, + // Security sales are reported under Internal purchases, + // where their proceeds can be read against what they + // funded. Only a vanished cash line lands here. + .lot_removed, .drip_negative => if (!isSaleKind(c)) { try printFlaggedLine(out, c, color, warn_color); }, .unmatched_transfer => { @@ -3219,6 +3488,69 @@ fn printCashFundedResidualLine(out: *std.Io.Writer, c: Change, color: bool, pos: ); } +/// Is this Change a security sale, i.e. something that released funds +/// into its account? Cash lines that drain are movement within the +/// cash pool, not a sale, so they are excluded. +fn isSaleKind(c: Change) bool { + return switch (c.kind) { + .position_closed, .lot_removed => c.security_type != .cash, + .drip_negative => true, + else => false, + }; +} + +/// Print the sales that released funds, one line per (account, symbol) +/// rather than one per lot. +/// +/// Collapsing is not cosmetic. A DRIP-fed position accumulates a lot +/// per distribution, so closing one can retire hundreds of records - +/// the trade that motivated this work retired 219, which as individual +/// lines swamped every other section in the report. The lot count is +/// kept in the output so nothing is hidden, just summarized. +fn printCollapsedSales(out: *std.Io.Writer, report: *const Report, color: bool, muted: [3]u8) !void { + for (report.changes, 0..) |c, i| { + if (!isSaleKind(c)) continue; + + // Print each group once, at its first member. Quadratic, but + // bounded by the sale count in a single window and it keeps + // this printer allocation-free like every other one here. + var already_printed = false; + for (report.changes[0..i]) |o| { + if (!isSaleKind(o)) continue; + if (std.mem.eql(u8, o.account, c.account) and std.mem.eql(u8, o.symbol, c.symbol)) { + already_printed = true; + break; + } + } + if (already_printed) continue; + + var proceeds: f64 = 0; + var lots: usize = 0; + var closed_in_place = false; + for (report.changes) |o| { + if (!isSaleKind(o)) continue; + if (!std.mem.eql(u8, o.account, c.account)) continue; + if (!std.mem.eql(u8, o.symbol, c.symbol)) continue; + proceeds += if (o.kind == .drip_negative) @abs(o.value()) else o.face_value; + lots += 1; + if (o.kind == .position_closed) closed_in_place = true; + } + + const acct = if (c.account.len == 0) "(no account)" else c.account; + try cli.setFg(out, color, muted); + // `close_price` is what the sale realized; anything else is a + // current-price proxy, so say which one the reader is looking at. + const basis: []const u8 = if (closed_in_place) "at close" else "at mark"; + if (lots == 1) { + try out.print(" {s:<14}{s:<24} sold {s} ({f})", .{ c.symbol, acct, basis, Money.from(proceeds) }); + } else { + try out.print(" {s:<14}{s:<24} sold {d} lots {s} ({f})", .{ c.symbol, acct, lots, basis, Money.from(proceeds) }); + } + try cli.reset(out, color); + try out.writeAll("\n"); + } +} + /// Render an "Internal purchases" row: a `new_stock` / `new_cd` lot /// funded (wholly or in part) by a same-account cash decrease. Muted - /// these don't count toward attribution. Shows the funded amount and, @@ -3959,6 +4291,668 @@ test "computeReport: transfer record takes priority over intra-account netting" } } +// ── Security-sale funding (intra-account reallocation) ─────── +// +// Case labels A-F below match the scenario table worked out when this +// was designed; each is a distinct shape of "did money actually enter +// the portfolio". They are kept together so the whole matrix is +// visible at once - the bug these fix was caused by reasoning about +// one shape (a sale in the same window as the rebuy) without checking +// the others. + +test "computeReport: case A - intra-account reallocation is not a contribution" { + // The reported bug, reduced. A bond fund is sold and the proceeds + // are immediately redeployed into two ETFs in the SAME account. + // Nothing entered the portfolio, so contributions must be $0. + // + // Shapes match the real trade: 41,285.219 sh of the sold fund at + // $11.32 = $467,348.68 of proceeds, redeployed as $467,639.27 of + // new lots, with the $290.59 difference topped up from the + // account's own cash. Every dollar is internal. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("OLDFUND", 11.32); + + const before = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 41_285.219, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage", .drip = true }, + .{ .symbol = "", .shares = 309.57, .open_date = Date.fromYmd(2026, 2, 25), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "NEWA", .shares = 13_747, .open_date = Date.fromYmd(2026, 8, 11), .open_price = 23.25, .account = "Sample Brokerage" }, + .{ .symbol = "NEWA", .shares = 1_436, .open_date = Date.fromYmd(2026, 8, 13), .open_price = 23.32, .account = "Sample Brokerage" }, + .{ .symbol = "NEWB", .shares = 1_800, .open_date = Date.fromYmd(2026, 8, 11), .open_price = 63.63, .account = "Sample Brokerage" }, + .{ .symbol = "", .shares = 18.98, .open_date = Date.fromYmd(2026, 2, 25), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 8, 17), .{}); + + var sale: ?Change = null; + var new_total: f64 = 0; + var attributed_total: f64 = 0; + for (report.changes) |c| switch (c.kind) { + .lot_removed => sale = c, + .new_stock => { + new_total += c.value(); + attributed_total += c.attributedValue(); + }, + else => {}, + }; + + // The sale is valued, and is an outflow - never a negative contribution. + try std.testing.expect(sale != null); + try std.testing.expectApproxEqAbs(@as(f64, 467_348.68), sale.?.face_value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, -467_348.68), sale.?.value(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), sale.?.attributedValue(), 0.01); + + // Proceeds plus the $290.59 of account cash fund the buys exactly. + try std.testing.expectApproxEqAbs(@as(f64, 467_639.27), new_total, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), attributed_total, 0.01); +} + +test "computeReport: case B - sale proceeds resting in cash are not a contribution" { + // Window 1 of a two-window reallocation: the position is sold and + // the proceeds just sit in the account's cash line. On an account + // WITHOUT cash_is_contribution the increase is plain cash_delta, + // which never counted - this pins that it stays that way now the + // sale carries dollars. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SYM", 100.0); + + const before = [_]Lot{ + .{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample Brokerage" }, + .{ .symbol = "", .shares = 500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "", .shares = 100_500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{}); + + var total: f64 = 0; + for (report.changes) |c| switch (c.kind) { + .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => total += c.attributedValue(), + else => {}, + }; + try std.testing.expectApproxEqAbs(@as(f64, 0.0), total, 0.01); +} + +test "computeReport: case C - cash from a prior window's sale funds the rebuy" { + // Window 2: the cash parked in window 1 buys the replacement. + // Already worked before this change (a negative cash_delta funds + // the buy); pinned so the new proceeds path cannot regress it. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const before = [_]Lot{ + .{ .symbol = "", .shares = 100_500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "NEWSYM", .shares = 2_000, .open_date = Date.fromYmd(2026, 5, 10), .open_price = 50.0, .account = "Sample Brokerage" }, + .{ .symbol = "", .shares = 500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 11), .{}); + + var total: f64 = 0; + for (report.changes) |c| switch (c.kind) { + .new_stock, .new_cash, .new_cd, .new_option, .cash_contribution => total += c.attributedValue(), + else => {}, + }; + try std.testing.expectApproxEqAbs(@as(f64, 0.0), total, 0.01); +} + +test "computeReport: case D - a sale resting in cash must not swallow a real deposit" { + // The case that makes the spent/resting split load-bearing. In one + // window: SYM is sold and the proceeds stay in cash, AND a + // separate $10k of new money arrives and is invested in VTI. + // + // A naive "sale proceeds fund new lots" budget would let the + // $100k sale absorb the $10k VTI purchase and report $0. The + // proceeds are demonstrably still in cash (the pool went UP), so + // they are unavailable for funding and the $10k must survive. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SYM", 100.0); + + const before = [_]Lot{ + .{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample Brokerage" }, + .{ .symbol = "", .shares = 1_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + // $100k proceeds land in cash; the $10k deposit passes through + // and out again into VTI, so the pool ends up +$100k. + .{ .symbol = "", .shares = 101_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + .{ .symbol = "VTI", .shares = 40, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 250.0, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{}); + + var vti: ?Change = null; + for (report.changes) |c| { + if (c.kind == .new_stock and std.mem.eql(u8, c.symbol, "VTI")) vti = c; + } + try std.testing.expect(vti != null); + try std.testing.expectApproxEqAbs(@as(f64, 10_000.0), vti.?.value(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), vti.?.internal_funded, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 10_000.0), vti.?.attributedValue(), 0.01); +} + +test "computeReport: case E/F - deposits still count, parked or invested" { + // Guard rails on the other side: with no sale anywhere, a deposit + // is a contribution whether it is left as cash (E) or spent on a + // security in the same window (F). + 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(); + + // E: cash pool grows, nothing bought. Plain cash_delta on an + // account without the opt-in flag, so it is deliberately NOT + // counted as a contribution - only the flag makes cash count. + const before_e = [_]Lot{ + .{ .symbol = "", .shares = 1_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after_e = [_]Lot{ + .{ .symbol = "", .shares = 11_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const rep_e = try computeReport(allocator, &before_e, &after_e, &prices, Date.fromYmd(2026, 5, 4), .{}); + var cash_e: ?Change = null; + for (rep_e.changes) |c| { + if (c.kind == .cash_delta) cash_e = c; + } + try std.testing.expect(cash_e != null); + try std.testing.expectApproxEqAbs(@as(f64, 10_000.0), cash_e.?.value(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), cash_e.?.internal_funded, 0.01); + + // F: deposit arrives and is invested in the same window - cash + // ends flat, the new lot is real new money. + const before_f = [_]Lot{ + .{ .symbol = "", .shares = 1_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after_f = [_]Lot{ + .{ .symbol = "", .shares = 1_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + .{ .symbol = "VTI", .shares = 40, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 250.0, .account = "Sample Brokerage" }, + }; + const rep_f = try computeReport(allocator, &before_f, &after_f, &prices, Date.fromYmd(2026, 5, 4), .{}); + var vti: ?Change = null; + for (rep_f.changes) |c| { + if (c.kind == .new_stock) vti = c; + } + try std.testing.expect(vti != null); + try std.testing.expectApproxEqAbs(@as(f64, 10_000.0), vti.?.attributedValue(), 0.01); +} + +test "computeReport: a partial sale funds a same-account purchase" { + // Shares reduced on an unchanged lot key (drip_negative) is still + // a sale, so its proceeds are a funding source too. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SYM", 100.0); + + const before = [_]Lot{ + .{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + // 200 shares sold at $100 = $20k, redeployed into NEWSYM. + .{ .symbol = "SYM", .shares = 800, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample Brokerage" }, + .{ .symbol = "NEWSYM", .shares = 400, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 50.0, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{}); + + var partial: ?Change = null; + var buy: ?Change = null; + for (report.changes) |c| switch (c.kind) { + .drip_negative => partial = c, + .new_stock => buy = c, + else => {}, + }; + try std.testing.expect(partial != null); + try std.testing.expect(buy != null); + try std.testing.expectApproxEqAbs(@as(f64, -20_000.0), partial.?.value(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), partial.?.attributedValue(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 20_000.0), buy.?.internal_funded, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.attributedValue(), 0.01); +} + +test "computeReport: a sale does NOT fund a purchase in a different account" { + // Same-account only. Selling in one account cannot silently + // explain a purchase in another - that is a transfer, and it needs + // a transaction_log.srf record. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SYM", 100.0); + + const before = [_]Lot{ + .{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample IRA" }, + }; + const after = [_]Lot{ + .{ .symbol = "NEWSYM", .shares = 2_000, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 50.0, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{}); + + var buy: ?Change = null; + for (report.changes) |c| { + if (c.kind == .new_stock) buy = c; + } + try std.testing.expect(buy != null); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.internal_funded, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 100_000.0), buy.?.attributedValue(), 0.01); +} + +test "computeReport: sale proceeds cancel a cash_is_contribution credit" { + // The latent bug. On an account flagged cash_is_contribution::true + // a positive cash delta is assumed to be new money - true for a + // payroll accrual, false for the proceeds of a sale, and the flag + // cannot tell them apart. Without the resting-proceeds drawdown + // this books a $100k sale as a $100k contribution. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("SYM", 100.0); + + var am = try analysis.parseAccountsFile(allocator, + \\#!srfv1 + \\account::Sample ESPP,tax_type::taxable,cash_is_contribution:bool:true + ); + defer am.deinit(); + + const before = [_]Lot{ + .{ .symbol = "SYM", .shares = 1_000, .open_date = Date.fromYmd(2025, 1, 2), .open_price = 60.0, .account = "Sample ESPP" }, + .{ .symbol = "", .shares = 500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample ESPP" }, + }; + const after = [_]Lot{ + // $100k of proceeds land in the cash line, plus $2k of genuine + // payroll accrual that the flag exists to catch. + .{ .symbol = "", .shares = 102_500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample ESPP" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ + .account_map = &am, + }); + + var credit: ?Change = null; + for (report.changes) |c| { + if (c.kind == .cash_contribution) credit = c; + } + try std.testing.expect(credit != null); + // Raw cash arrival is $102k; $100k of it is sale proceeds, so only + // the $2k accrual is a real contribution. + try std.testing.expectApproxEqAbs(@as(f64, 102_000.0), credit.?.value(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 100_000.0), credit.?.internal_funded, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 2_000.0), credit.?.attributedValue(), 0.01); +} + +test "computeReport: cash_is_contribution credit survives when there is no sale" { + // The other half of the pin above: with no sale in the window the + // flag must still credit the full cash arrival, or the opt-in + // stops doing its job. + 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(); + + var am = try analysis.parseAccountsFile(allocator, + \\#!srfv1 + \\account::Sample ESPP,tax_type::taxable,cash_is_contribution:bool:true + ); + defer am.deinit(); + + const before = [_]Lot{ + .{ .symbol = "", .shares = 500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample ESPP" }, + }; + const after = [_]Lot{ + .{ .symbol = "", .shares = 2_500, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample ESPP" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{ + .account_map = &am, + }); + + var credit: ?Change = null; + for (report.changes) |c| { + if (c.kind == .cash_contribution) credit = c; + } + try std.testing.expect(credit != null); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), credit.?.internal_funded, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 2_000.0), credit.?.attributedValue(), 0.01); +} + +// ── Positions closed in place (`position_closed`) ───────────── + +test "computeReport: a lot closed in place is a sale valued at close_price" { + // Archiving a sold lot - into a sibling `portfolio_closed.srf` that + // the same glob merges, or edited where it sits - keeps the record + // and adds close_date/close_price. The strict lot key excludes + // close_date and the share count is untouched, so without an + // explicit close check this emitted nothing at all and the sale + // vanished from attribution. + // + // close_price is authoritative: $11.32 realized against an $8.90 + // basis. Valuing at basis would leave the difference as a phantom + // contribution. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("OLDFUND", 9.00); // deliberately NOT the close price + + const before = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 41_285.219, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 41_285.219, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage", .close_date = Date.fromYmd(2026, 8, 10), .close_price = 11.32 }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 8, 17), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + const sale = report.changes[0]; + try std.testing.expectEqual(ChangeKind.position_closed, sale.kind); + // 41,285.219 x $11.32 - close_price wins over both the current + // price in the map and open_price. + try std.testing.expectApproxEqAbs(@as(f64, 467_348.68), sale.face_value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), sale.attributedValue(), 0.01); +} + +test "computeReport: closing a position funds a same-account repurchase" { + // The full reallocation as it looks once closed lots are retained + // rather than deleted: one position_closed plus the replacement + // buy, netting to zero new money. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const before = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 1_000, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 1_000, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage", .close_date = Date.fromYmd(2026, 8, 10), .close_price = 11.32 }, + .{ .symbol = "NEWA", .shares = 486, .open_date = Date.fromYmd(2026, 8, 11), .open_price = 23.29, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 8, 17), .{}); + + var buy: ?Change = null; + for (report.changes) |c| { + if (c.kind == .new_stock) buy = c; + } + try std.testing.expect(buy != null); + // $11,320 of proceeds cover the $11,318.94 purchase in full. + try std.testing.expectApproxEqAbs(@as(f64, 11_318.94), buy.?.value(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.attributedValue(), 0.01); +} + +test "computeReport: a close with no close_price falls back to a price proxy" { + // close_price is optional in the schema. Without it the sale still + // has to be valued or it funds nothing, so fall back to the same + // current-price proxy a deleted lot gets - never to open_price, + // which would understate a winner. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("OLDFUND", 11.00); + + const before = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 1_000, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "OLDFUND", .shares = 1_000, .open_date = Date.fromYmd(2021, 9, 6), .open_price = 8.90, .account = "Sample Brokerage", .close_date = Date.fromYmd(2026, 8, 10) }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 8, 17), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.position_closed, report.changes[0].kind); + try std.testing.expectApproxEqAbs(@as(f64, 11_000.0), report.changes[0].face_value, 0.01); +} + +test "computeReport: a close that also moves price:: is a sale, not a price edit" { + // Ordering pin. The same-shares branch checks `price::` too, and a + // reconciliation pass can easily stamp a final price on the way + // out. If the price comparison ran first this would classify as + // price_only, emit no outflow, and leave the repurchase looking + // like new money. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const before = [_]Lot{ + .{ .symbol = "MFUND", .shares = 100, .open_date = Date.fromYmd(2024, 1, 2), .open_price = 10.0, .price = 12.0, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "MFUND", .shares = 100, .open_date = Date.fromYmd(2024, 1, 2), .open_price = 10.0, .price = 13.0, .account = "Sample Brokerage", .close_date = Date.fromYmd(2026, 8, 10), .close_price = 13.0 }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 8, 17), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.position_closed, report.changes[0].kind); + try std.testing.expectApproxEqAbs(@as(f64, 1_300.0), report.changes[0].face_value, 0.01); +} + +test "computeReport: an option outflow is valued per contract, not per share" { + // Option lots price at open_price x multiplier (100 shares per + // contract), so an outflow that skipped the multiplier would + // undervalue the proceeds 100x and fund almost nothing. + 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{ + // 5 contracts at $3.20 = $1,600. + .{ .symbol = "SYM 260918C00050000", .shares = 5, .open_date = Date.fromYmd(2026, 5, 1), .open_price = 3.20, .security_type = .option, .account = "Sample Brokerage" }, + // A closed-in-place option, valued off close_price: 2 x $4.50 x 100. + .{ .symbol = "SYM 261218C00060000", .shares = 2, .open_date = Date.fromYmd(2026, 5, 1), .open_price = 2.00, .security_type = .option, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "SYM 261218C00060000", .shares = 2, .open_date = Date.fromYmd(2026, 5, 1), .open_price = 2.00, .security_type = .option, .account = "Sample Brokerage", .close_date = Date.fromYmd(2026, 8, 10), .close_price = 4.50 }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 8, 17), .{}); + + var removed: ?Change = null; + var closed: ?Change = null; + for (report.changes) |c| switch (c.kind) { + .lot_removed => removed = c, + .position_closed => closed = c, + else => {}, + }; + try std.testing.expect(removed != null); + try std.testing.expectApproxEqAbs(@as(f64, 1_600.0), removed.?.face_value, 0.01); + try std.testing.expect(closed != null); + try std.testing.expectApproxEqAbs(@as(f64, 900.0), closed.?.face_value, 0.01); +} + +test "computeReport: a cash lot closed in place drains the pool, it is not a sale" { + // Closing a cash line rather than deleting it still means the cash + // left. It must reduce the account's cash pool, not masquerade as + // sale proceeds - otherwise the same dollars would be counted as + // both an outflow and a funding source, funding the buy twice. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const before = [_]Lot{ + .{ .symbol = "", .shares = 30_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" }, + }; + const after = [_]Lot{ + .{ .symbol = "", .shares = 30_000, .open_date = Date.fromYmd(2026, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage", .close_date = Date.fromYmd(2026, 5, 3) }, + .{ .symbol = "SYM", .shares = 60, .open_date = Date.fromYmd(2026, 5, 3), .open_price = 500, .account = "Sample Brokerage" }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 5, 4), .{}); + + var buy: ?Change = null; + for (report.changes) |c| { + if (c.kind == .new_stock) buy = c; + } + try std.testing.expect(buy != null); + try std.testing.expectApproxEqAbs(@as(f64, 30_000.0), buy.?.internal_funded, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.attributedValue(), 0.01); +} + +test "printCollapsedSales: many retired lots render as one line" { + // A DRIP-fed position closes as one lot per historical + // distribution. The trade that motivated this retired 219 of them; + // printed individually they buried every other section. + 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(); + + var before: std.ArrayList(Lot) = .empty; + var after: std.ArrayList(Lot) = .empty; + for (0..40) |i| { + const day: u8 = @intCast(1 + i % 28); + const lot: Lot = .{ .symbol = "DRIPX", .shares = 10, .open_date = Date.fromYmd(2020, 1, day), .open_price = 9.0 + @as(f64, @floatFromInt(i)) * 0.01, .account = "Sample Brokerage" }; + try before.append(allocator, lot); + var closed = lot; + closed.close_date = Date.fromYmd(2026, 8, 10); + closed.close_price = 12.0; + try after.append(allocator, closed); + } + + const report = try computeReport(allocator, before.items, after.items, &prices, Date.fromYmd(2026, 8, 17), .{}); + try std.testing.expectEqual(@as(usize, 40), report.changes.len); + + var aw: std.Io.Writer.Allocating = .init(allocator); + try printReport(&aw.writer, &report, "test window", false); + const text = aw.written(); + + // Exactly one DRIPX row, carrying the lot count and the total. + var rows: usize = 0; + var it = std.mem.splitScalar(u8, text, '\n'); + while (it.next()) |line| { + if (std.mem.indexOf(u8, line, "DRIPX") != null) rows += 1; + } + try std.testing.expectEqual(@as(usize, 1), rows); + try std.testing.expect(std.mem.indexOf(u8, text, "sold 40 lots at close") != null); + try std.testing.expect(std.mem.indexOf(u8, text, "$4,800.00") != null); +} + +test "prepareReport: a lot archived into a sibling portfolio file is one sale" { + // End-to-end pin on the multi-file diff. The documented workflow for + // a sale is to move the closed lot out of `portfolio.srf` into a + // sibling `portfolio_closed.srf`, which the `portfolio*.srf` glob + // picks up. + // + // Reading only the first file - what this used to do - showed the + // deletion and nothing else, so the sale's proceeds were invisible + // and the repurchase read as a fresh contribution. Reading the + // merged union instead sees the lot gain close_date/close_price, + // and prices the sale off what it actually realized. + if (!test_git.available(std.testing.allocator)) return; + + const io = std.testing.io; + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPathFile(io, ".", &path_buf); + const dir = path_buf[0..dir_len]; + + // Before: the position is open, and an empty archive file already + // exists so the glob resolves the same on both sides. + try tmp.dir.writeFile(io, .{ .sub_path = "portfolio.srf", .data = + \\#!srfv1 + \\symbol::OLDFUND,shares:num:1000,open_date::2021-09-06,open_price:num:8.90,account::Sample Brokerage + \\ + }); + try tmp.dir.writeFile(io, .{ .sub_path = "portfolio_closed.srf", .data = "#!srfv1\n" }); + try test_git.run(allocator, dir, null, &.{ "init", "-q" }); + try test_git.run(allocator, dir, null, &.{ "config", "user.email", "test@example.com" }); + try test_git.run(allocator, dir, null, &.{ "config", "user.name", "Test" }); + try test_git.run(allocator, dir, null, &.{ "config", "commit.gpgsign", "false" }); + try test_git.run(allocator, dir, null, &.{ "add", "portfolio.srf", "portfolio_closed.srf" }); + try test_git.run(allocator, dir, "2026-08-01T12:00:00", &.{ "commit", "-q", "-m", "open" }); + + // After: sold at 11.32 and archived; proceeds redeployed into NEWA. + try tmp.dir.writeFile(io, .{ .sub_path = "portfolio.srf", .data = + \\#!srfv1 + \\symbol::NEWA,shares:num:486,open_date::2026-08-11,open_price:num:23.29,account::Sample Brokerage + \\ + }); + try tmp.dir.writeFile(io, .{ .sub_path = "portfolio_closed.srf", .data = + \\#!srfv1 + \\symbol::OLDFUND,close_date::2026-08-10,close_price:num:11.32,shares:num:1000,open_date::2021-09-06,open_price:num:8.90,account::Sample Brokerage + \\ + }); + try test_git.run(allocator, dir, null, &.{ "add", "portfolio.srf", "portfolio_closed.srf" }); + try test_git.run(allocator, dir, "2026-08-15T12:00:00", &.{ "commit", "-q", "-m", "sold" }); + + var env = try std.testing.environ.createMap(allocator); + defer env.deinit(); + + const open_path = try std.fs.path.join(allocator, &.{ dir, "portfolio.srf" }); + const closed_path = try std.fs.path.join(allocator, &.{ dir, "portfolio_closed.srf" }); + // Lexicographic order, matching how the glob resolves ('.' < '_'). + const paths: []const []const u8 = &.{ open_path, closed_path }; + + var svc = zfin.DataService.init(io, std.testing.allocator, .{ .cache_dir = dir }); + defer svc.deinit(); + + var ctx = prepareReport(io, std.testing.allocator, allocator, &env, &svc, paths, null, null, Date.fromYmd(2026, 8, 17), false, .never, .silent) catch return; + defer ctx.deinit(); + + var sale: ?Change = null; + var buy: ?Change = null; + for (ctx.report.changes) |c| switch (c.kind) { + .position_closed => sale = c, + .new_stock => buy = c, + else => {}, + }; + + // The archived lot reads as a close, not a disappearance... + try std.testing.expect(sale != null); + try std.testing.expectEqualStrings("OLDFUND", sale.?.symbol); + try std.testing.expectApproxEqAbs(@as(f64, 11_320.0), sale.?.face_value, 0.01); + // ...and its proceeds fund the repurchase, so nothing is new money. + try std.testing.expect(buy != null); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), buy.?.attributedValue(), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 0.0), summarizeAttribution(ctx).total(), 0.01); +} + test "computeReport: stock open_price renormalized reclassified as edit" { // Reconciliation tweak: user updates `open_price` to match the // institutional-share-class NAV, leaving everything else alone. @@ -4674,7 +5668,7 @@ test "resolveEndpoints: legacy dirty -> HEAD vs working copy" { defer env.deinit(); const repo: git.RepoInfo = .{ .root = "/tmp", .rel_path = "portfolio.srf" }; - const eps = try resolveEndpoints(std.testing.io, arena_state.allocator(), &env, repo, null, null, true, .verbose); + const eps = try resolveEndpoints(std.testing.io, arena_state.allocator(), &env, repo, &.{repo.rel_path}, null, null, true, .verbose); try std.testing.expectEqualStrings("HEAD", eps.range.before_rev); try std.testing.expect(eps.range.after_rev == null); try std.testing.expect(std.mem.indexOf(u8, eps.label, "working copy against HEAD") != null); @@ -4687,7 +5681,7 @@ test "resolveEndpoints: legacy clean -> HEAD~1 vs HEAD" { defer env.deinit(); const repo: git.RepoInfo = .{ .root = "/tmp", .rel_path = "portfolio.srf" }; - const eps = try resolveEndpoints(std.testing.io, arena_state.allocator(), &env, repo, null, null, false, .verbose); + const eps = try resolveEndpoints(std.testing.io, arena_state.allocator(), &env, repo, &.{repo.rel_path}, null, null, false, .verbose); try std.testing.expectEqualStrings("HEAD~1", eps.range.before_rev); try std.testing.expectEqualStrings("HEAD", eps.range.after_rev.?); try std.testing.expect(std.mem.indexOf(u8, eps.label, "HEAD~1 against HEAD") != null); @@ -5638,7 +6632,12 @@ test "printReport: in-kind transfer renders in Transfers section, out of totals" try std.testing.expect(std.mem.indexOf(u8, text, "New contributions / purchases") != null); try std.testing.expect(std.mem.indexOf(u8, text, "Transfers (matched - not counted)") != null); try std.testing.expect(std.mem.indexOf(u8, text, "Cash deltas") != null); - try std.testing.expect(std.mem.indexOf(u8, text, "Flagged for review") != null); + // The OLDX disposal is a security sale, so it reports as released + // funds under Internal purchases rather than as an anomaly under + // Flagged for review. + try std.testing.expect(std.mem.indexOf(u8, text, "Internal purchases") != null); + try std.testing.expect(std.mem.indexOf(u8, text, "OLDX") != null); + try std.testing.expect(std.mem.indexOf(u8, text, "Flagged for review") == null); try std.testing.expect(std.mem.indexOf(u8, text, "Summary by account") != null); try std.testing.expect(std.mem.indexOf(u8, text, "Grand total") != null); @@ -6590,7 +7589,7 @@ test "printReport: full report renders every section and sub-printer" { try testing.expect(std.mem.indexOf(u8, out, "rest from transfer") != null); // printPartialTransferLine try testing.expect(std.mem.indexOf(u8, out, "price ") != null); // printPriceOnlyLine try testing.expect(std.mem.indexOf(u8, out, "manual edit") != null); // printFlaggedLine flagged - try testing.expect(std.mem.indexOf(u8, out, "lot removed") != null); // printFlaggedLine lot_removed + try testing.expect(std.mem.indexOf(u8, out, "sold at mark") != null); // printCollapsedSales, no close_price try testing.expect(std.mem.indexOf(u8, out, "Transfer 2026-05-03") != null); // printUnmatchedTransferLine try testing.expect(std.mem.indexOf(u8, out, "unmatched wire") != null); try testing.expect(std.mem.indexOf(u8, out, "(no account)") != null); // summary no-account label diff --git a/src/git.zig b/src/git.zig index d95ccb2..8055f22 100644 --- a/src/git.zig +++ b/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 ` / `--until ` 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, diff --git a/src/portfolio_loader.zig b/src/portfolio_loader.zig index 78402fd..aa36208 100644 --- a/src/portfolio_loader.zig +++ b/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: {