diff --git a/docs/explanation/faq-troubleshooting.md b/docs/explanation/faq-troubleshooting.md index a4133f7..b65b30e 100644 --- a/docs/explanation/faq-troubleshooting.md +++ b/docs/explanation/faq-troubleshooting.md @@ -99,6 +99,24 @@ Declare it in it isn't counted as new money. See [Track contributions](../guides/track-contributions.md#dont-double-count-transfers). +**`compare` / `contributions` shows a big new lot I never bought, and +`audit` flags it under "Large new lots".** +Did you rename an account? The account name is zfin's only identifier +for an account, and it's part of each lot's identity key -- so renaming +one makes every lot in it look like it was closed and re-bought +elsewhere. It's cosmetic and happens only for the one review window +that straddles the rename commit; nothing is corrupted. See +[Renaming an account](../guides/set-up-accounts.md#renaming-an-account) +for the full list of consequences, and why account names should be +chosen to never change. + +**I want to see my tax-loss-harvested total, but the account is a +synthetic proxy so zfin can't compute it.** +Record it by hand with +[`harvested` / `harvested_date`](../reference/config/accounts-srf.md#harvested-and-harvested_date) +in `accounts.srf`. Don't encode it in the account name -- see the entry +above for why. + ## See also - [Core concepts](concepts.md) -- the mental model. diff --git a/docs/guides/set-up-accounts.md b/docs/guides/set-up-accounts.md index af9a299..274b03d 100644 --- a/docs/guides/set-up-accounts.md +++ b/docs/guides/set-up-accounts.md @@ -67,7 +67,7 @@ account::Old Rollover,tax_type::traditional,update_cadence::none ## 4. Advanced flags -Three optional fields change how analysis and the audit treat an +Four optional fields change how analysis and the audit treat an account -- see the reference for details: - **`shielded:bool:false`** -- mark a pre-tax account that is *not* @@ -79,6 +79,9 @@ account -- see the reference for details: account as real external contributions in [`zfin contributions`](track-contributions.md), instead of internal noise. +- **`direct_indexing:bool:true`** -- mark an account whose lots track a + benchmark with tracking-error drift rather than holding it directly, + so routine share reconciliation stops registering as money movement. - **`audit_large_lot_threshold:num:50000`** -- raise (or lower) the dollar cutoff at which a flagless [`zfin audit`](../reference/cli/audit.md) nudges you to confirm a **new lot**'s source. The default is $10,000; @@ -89,6 +92,98 @@ account -- see the reference for details: account::Sample ESPP,tax_type::taxable,audit_large_lot_threshold:num:50000 ``` +## 5. Track tax-loss harvested on a synthetic account + +If you run a tax-loss-harvesting sleeve (direct indexing, a managed +separate account), you probably don't want its hundreds of churning +positions in `portfolio.srf`. The usual shape is one aggregate lot with +a `ticker::` alias standing in for the whole sleeve: + +```srf +symbol::DI-SPX,ticker::SPY,shares:num:1000,open_date::2024-01-15,open_price:num:400,account::Sample Tax Loss +``` + +That keeps the file readable, but it means zfin has no closed lots to +work from, so it can't compute realized gain/loss -- and the harvested +total is the whole reason the account exists. Record it by hand +instead: + +```srf +account::Sample Tax Loss,tax_type::taxable,direct_indexing:bool:true,harvested:num:45300,harvested_date::2026-06-24 +``` + +Next time you check the figure on the brokerage site, update both +fields. It shows up as a compact annotation beside the account name in +`zfin analysis`, the TUI Analysis tab, the TUI account picker (`a`), +and the TUI Portfolio tab header when you filter to that account: + +``` + By Account + Sample Tax Loss █████████████████████████████ 96.7% $738,930.00 (45k 6/24) +``` + +The date is required -- an undated hand-copied number is +indistinguishable from a stale one, so zfin won't display it and +[`zfin doctor`](../reference/cli/doctor.md) will tell you. Entries more +than 12 months old stop displaying on purpose. The sign is ignored, so +`-45300` works too. + +**Don't put the number in the account name.** It's tempting -- the name +already prints in a lot of places -- but the name is a join key, not a +label. See the next section for what a rename costs you. + +## Renaming an account + +Renaming an account is a bigger deal than it looks, because the account +name is zfin's **only** identifier for an account. There is no separate +account ID. The name is matched byte-exactly against `account::` on +every lot, and it is written into every historical snapshot at the time +that snapshot is taken. + +To rename, change it in **both** `accounts.srf` and every `account::` +on your `portfolio.srf` lots, in a single commit. Then expect the +following: + +**One-time noise in contributions and compare.** zfin identifies a lot +by `(security_type, symbol, account, open_date, open_price)`. Changing +the account breaks that key on every lot in the account, so +[`zfin contributions`](track-contributions.md) and `zfin compare` see +the old lots vanish and identical new ones appear -- indistinguishable +from closing one account and opening another. Any review window +spanning the rename commit will report phantom new lots and an inflated +contribution total, and a flagless [`zfin audit`](../reference/cli/audit.md) +will list them under **Large new lots - confirm source**. This happens +exactly once, for the one window that straddles the rename. Nothing is +corrupted; just ignore that run. + +**Truncated audit-staleness history.** The cadence nag derives each +account's last-updated time by walking `portfolio.srf`'s git history. +Older commits only contain the old name, so the account's update +history restarts at the rename commit. The rename itself registers as +an update, so the account reads as freshly refreshed rather than +falsely overdue. + +**A permanent split in per-account snapshot history.** Snapshots +already on disk record the old name and are immutable historical +records -- zfin does not rewrite them. Any per-account historical series +therefore has a discontinuity at the rename date. This one does not +heal. + +**Possible renumbering in the TUI account picker.** Shortcut keys are +assigned by list position (`accounts.srf` order, then alphabetical for +anything not listed), so a name that sorts differently can shift the +keys. Cosmetic. + +`zfin import` is unaffected: it resolves accounts by +`(institution, account_number)` and looks the name up in +`accounts.srf`, so a consistent rename in both files keeps working. + +The practical advice: **pick account names that never need to change.** +Don't encode anything that varies -- balances, dates, harvested totals, +"current"/"old" markers. Values that change belong in fields +(see [`harvested`](../reference/config/accounts-srf.md#harvested-and-harvested_date)), +not in the key. + ## Example (from `examples/pre-retirement-both`) ```srf diff --git a/docs/reference/cli/doctor.md b/docs/reference/cli/doctor.md index 3d29347..04abd30 100644 --- a/docs/reference/cli/doctor.md +++ b/docs/reference/cli/doctor.md @@ -20,7 +20,11 @@ Four sections, each line tagged `OK` / `INFO` / `WARN` / `FAIL`: from, and does it parse? - **Cross-checks** -- do `accounts.srf` / `metadata.srf` / `transaction_log.srf` reference entries that actually exist (e.g. - every account used by a lot has an `accounts.srf` entry)? + every account used by a lot has an `accounts.srf` entry)? Also + validates the hand-maintained + [`harvested` / `harvested_date`](../config/accounts-srf.md#harvested-and-harvested_date) + pair: an amount with no date, or a date in the future, won't display, + so `doctor` says so. - **Environment** -- cache size, staleness of the hand-maintained data tables, and `ZFIN_SERVER` reachability and version. - **Capabilities** -- which API keys are set and what each enables (or @@ -56,6 +60,7 @@ Files Cross-checks [OK ] accounts.srf coverage: all referenced entries present + [INFO] accounts.srf harvested: no accounts declare a harvested figure [OK ] metadata.srf coverage: all referenced entries present [INFO] transaction_log.srf references: skipped (transaction_log.srf not loaded) diff --git a/docs/reference/config/accounts-srf.md b/docs/reference/config/accounts-srf.md index 45f91b8..fda43ec 100644 --- a/docs/reference/config/accounts-srf.md +++ b/docs/reference/config/accounts-srf.md @@ -35,6 +35,8 @@ account::Joint taxable,tax_type::taxable,institution::schwab,account_number::JT0 | `direct_indexing` | bool | No | `false` | Marks an account whose lots track a benchmark with tracking-error drift (loosens contribution/audit tolerances). | | `shielded` | bool | No | (derived) | Umbrella-exposure override (see below). | | `audit_large_lot_threshold` | num | No | `10000` | Per-account dollar cutoff for the audit "Large new lots" nudge (see below). Must be positive. | +| `harvested` | num | No | -- | Hand-declared cumulative tax-loss-harvested figure, for accounts whose realized P&L zfin cannot derive (see below). | +| `harvested_date` | date | No | -- | `YYYY-MM-DD` you last refreshed `harvested`. Required for it to display. | ## Tax types @@ -75,8 +77,70 @@ Accounts you don't list, or list without the field, use that default. The value must be **positive** -- zero or a negative number is rejected at load time and the account falls back to the default. -## `update_cadence` and the audit nag +## `harvested` and `harvested_date` +Some accounts hold a number that matters to you but that zfin has no +way to compute. The motivating case is a **synthetic direct-indexing +account**: a tax-loss-harvesting sleeve holding hundreds of individual +positions that churn constantly. Rather than model all of them, you +track the sleeve as a single aggregate lot with a `ticker::` alias: + +```srf +#!srfv1 +symbol::DI-SPX,ticker::SPY,shares:num:1000,open_date::2024-01-15,open_price:num:400,account::Sample Tax Loss +``` + +That keeps the portfolio readable, but it means there are no closed +lots, so zfin cannot derive realized gain/loss -- and the harvested +total is the entire point of such an account. `harvested` is where you +park the figure you read off the brokerage site: + +```srf +#!srfv1 +account::Sample Tax Loss,tax_type::taxable,direct_indexing:bool:true,harvested:num:45300,harvested_date::2026-06-24 +``` + +It renders as a compact annotation next to the account name: + +``` + By Account + Sample Tax Loss █████████████████████████████ 96.7% $738,930.00 (45k 6/24) +``` + +It appears in four places: + +- the **By Account** breakdown of [`zfin analysis`](../cli/analysis.md) +- the same breakdown in the TUI's Analysis tab +- the TUI's account picker (`a`), beside the account number +- the TUI Portfolio tab header when an account filter is active + +### Rules + +- **`harvested_date` is required for it to display.** A hand-copied + figure with no date is indistinguishable from a stale one, so zfin + refuses to show it. [`zfin doctor`](../cli/doctor.md) warns when + `harvested` is set without a date, and when a date is in the future. +- **Entries older than 12 months stop displaying.** Harvest data that + old isn't decision-useful. This is designed behavior, not an error -- + `doctor` stays quiet about it. Refresh the figure and the date to + bring it back. +- **The sign doesn't matter.** `harvested:num:45300` and + `harvested:num:-45300` are equivalent; the annotation's parentheses + already carry the accounting "this is a loss" convention. +- **It is display-only.** It never enters a total, a weight, a + breakdown value, or the contributions attribution. It is an + annotation, not an input. + +### Why not put it in the account name? + +Because the account name is a **join key**, not a label. It is matched +byte-exactly against `account::` on every lot, and it is written into +every historical snapshot. Encoding a changing number in it means +renaming the account every time the number changes, and every rename +costs you history -- see +[Renaming an account](../../guides/set-up-accounts.md#renaming-an-account). + +## `update_cadence` and the audit nag [`zfin audit`](../cli/audit.md) (run flagless) flags accounts you haven't refreshed within their cadence window: `weekly` = 7 days, `monthly` = 30, `quarterly` = 90, `none` = never nag. The default is diff --git a/src/analytics/analysis.zig b/src/analytics/analysis.zig index cf93ce0..019530d 100644 --- a/src/analytics/analysis.zig +++ b/src/analytics/analysis.zig @@ -10,6 +10,7 @@ const ClassificationMap = @import("../models/classification.zig").Classification const ClassificationEntry = @import("../models/classification.zig").ClassificationEntry; const Portfolio = @import("../models/portfolio.zig").Portfolio; const Date = @import("../Date.zig"); +const fmt = @import("../format.zig"); const log = std.log.scoped(.accounts); @@ -18,6 +19,48 @@ pub const BreakdownItem = struct { label: []const u8, value: f64, // dollar amount weight: f64, // fraction of total (0.0 - 1.0) + /// Optional compact suffix rendered after the dollar value. + /// Only the "By Account" breakdown populates it (see + /// `annotateAccountBreakdown`); every other breakdown leaves it + /// null. + annotation: ?Annotation = null, +}; + +/// Pre-rendered compact annotation for a breakdown row. +/// +/// A value type on purpose: `BreakdownItem.label` is *borrowed* (it +/// points into the aggregation map's keys, which point into lot / +/// accounts.srf strings) and `AnalysisResult.deinit` frees only the +/// item slices, never any strings. An allocated annotation would break +/// that invariant and force an asymmetric free loop over one of the +/// five breakdown slices. Inlining the bytes keeps "BreakdownItem owns +/// nothing" true. +pub const Annotation = struct { + /// Inline capacity in bytes. Sized for the longest annotation any + /// breakdown produces today - `format.harvest_annotation_max_len` + /// ("(999.9M 12/31)", 14 bytes) - rounded up with a little + /// headroom. `annotateAccountBreakdown` asserts the fit at comptime, + /// so widening a producer's format without widening this is a build + /// error rather than a silently-dropped annotation. + pub const capacity = 16; + + buf: [capacity]u8, + len: u8, + + /// Build from a rendered string. Returns null for an empty string + /// (the "nothing to render" signal from the formatters) so callers + /// can assign the result straight into `BreakdownItem.annotation`. + /// Also returns null rather than truncating when `s` doesn't fit. + pub fn from(s: []const u8) ?Annotation { + if (s.len == 0 or s.len > capacity) return null; + var a: Annotation = .{ .buf = @splat(0), .len = @intCast(s.len) }; + @memcpy(a.buf[0..s.len], s); + return a; + } + + pub fn slice(self: *const Annotation) []const u8 { + return self.buf[0..self.len]; + } }; /// Tax type classification for accounts. @@ -113,6 +156,37 @@ pub const AccountTaxEntry = struct { /// time (warned + treated as unset) since zero would flag every /// new lot and negative is meaningless. audit_large_lot_threshold: ?f64 = null, + /// Cumulative tax-loss-harvested figure for this account, declared + /// by hand because zfin cannot derive it. + /// + /// The motivating case is a synthetic direct-indexing account (see + /// `direct_indexing`): the portfolio models it as one aggregate lot + /// with a `ticker::` alias, deliberately NOT enumerating the + /// hundreds of underlying positions. Without per-position detail + /// there are no closed lots, so `Lot.realizedGainLoss` has nothing + /// to work from - yet the harvested total is the whole point of + /// such an account. This field is where you park the number you + /// read off the brokerage site. + /// + /// Display-only. It never enters any total, weight, or breakdown + /// value; it is rendered as a compact annotation next to the + /// account name (`format.fmtHarvestAnnotation`). Sign-insensitive + /// on read - `harvested:num:45300` and `harvested:num:-45300` are + /// equivalent, since the annotation's parens already carry the + /// accounting "this is a loss" convention. + /// + /// Requires `harvested_date` to render; see that field. + harvested: ?f64 = null, + /// The "as of" date for `harvested` - when you last copied the + /// figure from the brokerage. Required for the annotation to + /// render at all, because a hand-copied number with no date is + /// indistinguishable from a stale one. + /// + /// Entries older than 12 months stop rendering: harvest data that + /// old is not decision-useful, and the cutoff is what lets the + /// annotation show a bare `M/D` with no year (within a trailing + /// year, each month/day pair occurs at most once). + harvested_date: ?Date = null, }; /// Update cadence for manual account maintenance. Parsed from accounts.srf. @@ -230,11 +304,47 @@ pub const AccountMap = struct { } return null; } + + /// Hand-declared tax-loss-harvested figure for `account`, plus the + /// date it was last refreshed. See `AccountTaxEntry.harvested`. + /// + /// Returns null when the account isn't in the map or its entry + /// omits `harvested`. The amount is already sign-normalized by the + /// parser; `as_of` may still be null (the caller decides whether a + /// dateless figure is renderable - `format.fmtHarvestAnnotation` + /// says no). + pub fn harvestedFor(self: AccountMap, account: []const u8) ?struct { amount: f64, as_of: ?Date } { + for (self.entries) |e| { + if (std.mem.eql(u8, e.account, account)) { + const amt = e.harvested orelse return null; + return .{ .amount = amt, .as_of = e.harvested_date }; + } + } + return null; + } }; +/// Populate `items[*].annotation` for a "By Account" breakdown with each +/// account's hand-declared tax-loss-harvested figure. Rows for accounts +/// with no `harvested` entry (or a stale / future-dated / dateless one) +/// are left null and render nothing. +/// +/// `as_of` is the reference date for the 12-month staleness window, not +/// necessarily today: a back-dated analysis should not surface a harvest +/// figure recorded after the date being analyzed. +pub fn annotateAccountBreakdown(items: []BreakdownItem, account_map: AccountMap, as_of: Date) void { + comptime std.debug.assert(fmt.harvest_annotation_max_len <= Annotation.capacity); + for (items) |*item| { + const h = account_map.harvestedFor(item.label) orelse continue; + var buf: [fmt.harvest_annotation_max_len]u8 = undefined; + item.annotation = Annotation.from(fmt.fmtHarvestAnnotation(&buf, h.amount, h.as_of, as_of)); + } +} + /// Parse an accounts.srf file into an AccountMap. /// Each record has: account::,tax_type::[,institution::][,account_number::][,] -/// where the optional flags include `audit_large_lot_threshold:num:`. +/// where the optional flags include `audit_large_lot_threshold:num:`, +/// `harvested:num:` and `harvested_date::`. pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !AccountMap { var entries = std.ArrayList(AccountTaxEntry).empty; errdefer { @@ -266,6 +376,21 @@ pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !Accoun break :blk null; } else null; + // `harvested` is a magnitude: the annotation's parens carry the + // "this is a loss" convention, so accept either sign from the + // user and normalize. A non-finite value can only arrive from a + // hand-typed `inf`/`nan`; there is nothing sensible to display + // for it, so drop the field rather than render garbage. + const harvested: ?f64 = if (entry.harvested) |h| blk: { + if (std.math.isFinite(h)) break :blk @abs(h); + // Silent under `zig build test`: the parser's own tests feed + // non-finite values on purpose, and the warn spam pollutes + // test output. + if (!builtin.is_test) + log.warn("accounts.srf: account '{s}': harvested must be a finite number (got {d}); ignoring", .{ entry.account, h }); + break :blk null; + } else null; + try entries.append(allocator, .{ .account = try allocator.dupe(u8, entry.account), .tax_type = entry.tax_type, @@ -276,6 +401,8 @@ pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !Accoun .direct_indexing = entry.direct_indexing, .shielded = entry.shielded, .audit_large_lot_threshold = lot_threshold, + .harvested = harvested, + .harvested_date = entry.harvested_date, }); } @@ -797,11 +924,18 @@ pub fn analyzePortfolio( // Convert maps to sorted slices const total = if (total_portfolio_value > 0) total_portfolio_value else 1.0; + // Hoisted so the per-account harvested annotations can be stamped + // in before the result is returned. `analyzePortfolio` already has + // both inputs the annotation needs (`account_map` and `as_of`), so + // no renderer needs to learn about accounts.srf. + const account_items = try mapToSortedBreakdown(allocator, acct_map, total); + if (account_map) |am| annotateAccountBreakdown(account_items, am, as_of); + return .{ .asset_category = try mapToSortedBreakdown(allocator, asset_cat_map, total), .sector = try mapToSortedBreakdown(allocator, sector_map, total), .geo = try mapToSortedBreakdown(allocator, geo_map, total), - .account = try mapToSortedBreakdown(allocator, acct_map, total), + .account = account_items, .tax_type = try mapToSortedBreakdown(allocator, tax_map, total), .unclassified = try unclassified_list.toOwnedSlice(allocator), .total_value = total_portfolio_value, @@ -1062,6 +1196,163 @@ test "parseAccountsFile: non-positive audit_large_lot_threshold is rejected -> n } } +// ── harvested / harvested_date ─────────────────────────────── + +test "parseAccountsFile: harvested omitted -> harvestedFor returns null" { + const data = + \\#!srfv1 + \\account::Sample Brokerage,tax_type::taxable + ; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + + try std.testing.expect(am.entries[0].harvested == null); + try std.testing.expect(am.entries[0].harvested_date == null); + try std.testing.expect(am.harvestedFor("Sample Brokerage") == null); +} + +test "parseAccountsFile: harvested + harvested_date parse and look up by account" { + const data = + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,direct_indexing:bool:true,harvested:num:45300,harvested_date::2026-06-24 + \\account::Sample Brokerage,tax_type::taxable + ; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + + const h = am.harvestedFor("Sample Tax Loss").?; + try std.testing.expectApproxEqAbs(@as(f64, 45_300), h.amount, 0.001); + try std.testing.expect(h.as_of.?.eql(Date.fromYmd(2026, 6, 24))); + // Sibling account is unaffected. + try std.testing.expect(am.harvestedFor("Sample Brokerage") == null); + // Unknown account. + try std.testing.expect(am.harvestedFor("Nope") == null); +} + +test "parseAccountsFile: negative harvested is normalized to a magnitude" { + // The annotation's parens carry the "this is a loss" convention, so + // a user writing the figure as a negative gets the same result. + inline for (.{ "45300", "-45300" }) |written| { + const data = "#!srfv1\naccount::Sample Tax Loss,tax_type::taxable,harvested:num:" ++ written ++ ",harvested_date::2026-06-24\n"; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + try std.testing.expectApproxEqAbs(@as(f64, 45_300), am.harvestedFor("Sample Tax Loss").?.amount, 0.001); + } +} + +test "parseAccountsFile: zero harvested is kept (means nothing harvested yet)" { + const data = + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:0,harvested_date::2026-06-24 + ; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + + try std.testing.expectApproxEqAbs(@as(f64, 0), am.harvestedFor("Sample Tax Loss").?.amount, 0.001); +} + +test "parseAccountsFile: harvested without harvested_date parses but is not renderable" { + // Kept in the map (doctor nags about the missing date); the + // annotation formatter is what refuses to render it. + const data = + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300 + ; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + + const h = am.harvestedFor("Sample Tax Loss").?; + try std.testing.expectApproxEqAbs(@as(f64, 45_300), h.amount, 0.001); + try std.testing.expect(h.as_of == null); + + var buf: [fmt.harvest_annotation_max_len]u8 = undefined; + try std.testing.expectEqualStrings("", fmt.fmtHarvestAnnotation(&buf, h.amount, h.as_of, Date.fromYmd(2026, 7, 25))); +} + +test "parseAccountsFile: harvested_date without harvested is inert" { + const data = + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested_date::2026-06-24 + ; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + + try std.testing.expect(am.harvestedFor("Sample Tax Loss") == null); + try std.testing.expect(am.entries[0].harvested_date != null); +} + +test "parseAccountsFile: non-finite harvested is rejected -> null, account still parses" { + inline for (.{ "nan", "inf", "-inf" }) |bad| { + const data = "#!srfv1\naccount::Sample Tax Loss,tax_type::taxable,harvested:num:" ++ bad ++ ",harvested_date::2026-06-24\n"; + const allocator = std.testing.allocator; + var am = try parseAccountsFile(allocator, data); + defer am.deinit(); + try std.testing.expectEqual(@as(usize, 1), am.entries.len); + try std.testing.expectEqual(TaxType.taxable, am.entries[0].tax_type); + try std.testing.expect(am.harvestedFor("Sample Tax Loss") == null); + } +} + +test "Annotation.from: empty string yields null, round-trips otherwise" { + try std.testing.expect(Annotation.from("") == null); + // Longer than the inline buffer is dropped rather than truncated. + try std.testing.expect(Annotation.from("x" ** (Annotation.capacity + 1)) == null); + const a = Annotation.from("(45k 6/24)").?; + try std.testing.expectEqualStrings("(45k 6/24)", a.slice()); + // Exactly-buffer-sized input fits. + const full = Annotation.from("x" ** Annotation.capacity).?; + try std.testing.expectEqualStrings("x" ** Annotation.capacity, full.slice()); + // Every producer's widest output must fit - mirrored by the comptime + // assert in annotateAccountBreakdown. + try std.testing.expect(fmt.harvest_annotation_max_len <= Annotation.capacity); +} + +test "annotateAccountBreakdown: stamps only accounts with fresh harvested data" { + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2026-06-24 + \\account::Sample Stale,tax_type::taxable,harvested:num:12000,harvested_date::2024-01-15 + \\account::Sample Dateless,tax_type::taxable,harvested:num:9000 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + var items = [_]BreakdownItem{ + .{ .label = "Sample Tax Loss", .value = 412_300, .weight = 0.4 }, + .{ .label = "Sample Stale", .value = 100_000, .weight = 0.2 }, + .{ .label = "Sample Dateless", .value = 100_000, .weight = 0.2 }, + .{ .label = "Sample Brokerage", .value = 100_000, .weight = 0.1 }, + .{ .label = "Not In accounts.srf", .value = 50_000, .weight = 0.1 }, + }; + annotateAccountBreakdown(&items, am, Date.fromYmd(2026, 7, 25)); + + try std.testing.expectEqualStrings("(45k 6/24)", items[0].annotation.?.slice()); + try std.testing.expect(items[1].annotation == null); // >12 months old + try std.testing.expect(items[2].annotation == null); // no date + try std.testing.expect(items[3].annotation == null); // no harvested field + try std.testing.expect(items[4].annotation == null); // not in accounts.srf +} + +test "annotateAccountBreakdown: as_of is the reference date, not necessarily today" { + // A back-dated analysis must not surface a harvest figure recorded + // after the date being analyzed. + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2026-06-24 + ); + defer am.deinit(); + + var items = [_]BreakdownItem{.{ .label = "Sample Tax Loss", .value = 1000, .weight = 1.0 }}; + annotateAccountBreakdown(&items, am, Date.fromYmd(2026, 1, 1)); + try std.testing.expect(items[0].annotation == null); +} + // ── umbrellaExposure ───────────────────────────────────────── /// Helper: build an in-memory AccountMap from a literal SRF diff --git a/src/commands/analysis.zig b/src/commands/analysis.zig index 663cab6..5529e68 100644 --- a/src/commands/analysis.zig +++ b/src/commands/analysis.zig @@ -268,7 +268,13 @@ pub fn printBreakdownSection(out: *std.Io.Writer, items: []const zfin.analysis.B if (color) try fmt.ansiSetFg(out, cli.CLR_ACCENT[0], cli.CLR_ACCENT[1], cli.CLR_ACCENT[2]); try out.writeAll(bar); if (color) try fmt.ansiReset(out); - try out.print(" {d:>5.1}% {f}\n", .{ pct, Money.from(item.value) }); + try out.print(" {d:>5.1}% {f}", .{ pct, Money.from(item.value) }); + // Optional trailing annotation (the "By Account" breakdown's + // per-account harvested figure). Appended after the value so it + // can't disturb the label or bar alignment. Unstyled - the + // parens are the affordance. + if (item.annotation) |ann| try out.print(" {s}", .{ann.slice()}); + try out.print("\n", .{}); } } @@ -342,6 +348,38 @@ test "printBreakdownSection with color emits ANSI" { try std.testing.expect(std.mem.indexOf(u8, out, "\x1b[") != null); } +test "printBreakdownSection: annotation renders after the value" { + var buf: [4096]u8 = undefined; + var w: std.Io.Writer = .fixed(&buf); + const items = [_]zfin.analysis.BreakdownItem{ + .{ + .label = "Sample Tax Loss", + .weight = 0.40, + .value = 412_300.0, + .annotation = zfin.analysis.Annotation.from("(45k 6/24)"), + }, + }; + try printBreakdownSection(&w, &items, 24, 30, false); + const out = w.buffered(); + const value_at = std.mem.indexOf(u8, out, "$412,300.00").?; + const ann_at = std.mem.indexOf(u8, out, "(45k 6/24)").?; + try std.testing.expect(ann_at > value_at); + // Exactly one row, annotation last before the newline. + try std.testing.expect(std.mem.endsWith(u8, std.mem.trimEnd(u8, out, "\n"), "(45k 6/24)")); +} + +test "printBreakdownSection: null annotation adds nothing" { + var buf: [4096]u8 = undefined; + var w: std.Io.Writer = .fixed(&buf); + const items = [_]zfin.analysis.BreakdownItem{ + .{ .label = "Sample Brokerage", .weight = 0.40, .value = 412_300.0 }, + }; + try printBreakdownSection(&w, &items, 24, 30, false); + const out = w.buffered(); + try std.testing.expect(std.mem.indexOf(u8, out, "(") == null); + try std.testing.expect(std.mem.endsWith(u8, std.mem.trimEnd(u8, out, "\n"), "$412,300.00")); +} + test "display shows all sections" { var buf: [8192]u8 = undefined; var w: std.Io.Writer = .fixed(&buf); diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index e4a55f5..e5054ca 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -1191,13 +1191,20 @@ 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 - /// Strict lot key broke (open_date / open_price / account rewritten) - /// but the same (security_type, symbol, account) reappears on the - /// other side with approximately the same share total. Classified - /// as a lot edit - not counted as contribution, not counted as - /// disposal. Fixes the phantom-contribution bug from reconciliation - /// tweaks, CD auto-renewals rewriting `open_date`, and account - /// renames that shift every lot in the account under a new name. + /// 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. + /// Classified as a lot edit - not counted as contribution, not + /// counted as disposal. Fixes the phantom-contribution bug from + /// reconciliation tweaks, CD auto-renewals rewriting `open_date`, + /// and symbol-alias rewrites (`symbol::SPY` -> `symbol::DI-SPX, + /// ticker::SPY`). + /// + /// Deliberately does NOT cover account renames: `secondaryKey` + /// includes the account, so renaming one breaks both keys and the + /// rename is indistinguishable from closing one account and opening + /// another with the same positions. See the "account rename is NOT + /// collapsed" test below and the TODO.md entry for the known fix. lot_edited, price_only, // same key, price:: field changed, no share change flagged, // any other shape of edit @@ -4373,7 +4380,9 @@ test "computeReport: account rename is NOT collapsed (documented limitation)" { // one account, opening another with the same positions). Account // renames must therefore be handled manually: either avoid them // during a review window, or accept the phantom attribution for - // that week. Tracked in TODO.md. + // that week. Tracked in TODO.md; the user-facing consequences are + // documented in docs/guides/set-up-accounts.md ("Renaming an + // account"). var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const allocator = arena_state.allocator(); diff --git a/src/commands/doctor.zig b/src/commands/doctor.zig index 8ff2341..ffeb2cc 100644 --- a/src/commands/doctor.zig +++ b/src/commands/doctor.zig @@ -189,6 +189,74 @@ fn coverageCheck( }; } +/// Validate the hand-maintained `harvested` / `harvested_date` pair on +/// each `accounts.srf` entry. Pure over the parsed map so every branch +/// is unit-testable. +/// +/// Two failure modes, both hand-edit typos rather than corruption, so +/// both are WARN and neither stops anything: +/// +/// - `harvested` with no `harvested_date`: renders nothing, because a +/// hand-copied figure with no date is indistinguishable from a +/// stale one. Silently invisible is worse than a nag. +/// - `harvested_date` in the future relative to `as_of`: also renders +/// nothing, and is almost always a year typo. +/// +/// A `harvested_date` older than 12 months is NOT flagged - that's the +/// designed behavior (the annotation retires itself), not an error. +fn harvestedFieldChecks(arena: std.mem.Allocator, am: analysis.AccountMap, as_of: Date) !Check { + var undated: std.ArrayList([]const u8) = .empty; + var future: std.ArrayList([]const u8) = .empty; + var configured: usize = 0; + + for (am.entries) |e| { + if (e.harvested == null) continue; + configured += 1; + const on = e.harvested_date orelse { + try undated.append(arena, e.account); + continue; + }; + if (as_of.lessThan(on)) try future.append(arena, e.account); + } + + if (configured == 0) { + return .{ .status = .info, .label = "accounts.srf harvested", .detail = "no accounts declare a harvested figure" }; + } + if (undated.items.len == 0 and future.items.len == 0) { + return .{ + .status = .ok, + .label = "accounts.srf harvested", + .detail = try std.fmt.allocPrint(arena, "{d} account(s) with a dated harvested figure", .{configured}), + }; + } + if (undated.items.len > 0 and future.items.len > 0) { + return .{ + .status = .warn, + .label = "accounts.srf harvested", + .detail = try std.fmt.allocPrint(arena, "missing harvested_date: {s}; harvested_date in the future: {s}", .{ + try joinCapped(arena, undated.items, 6), + try joinCapped(arena, future.items, 6), + }), + }; + } + if (undated.items.len > 0) { + return .{ + .status = .warn, + .label = "accounts.srf harvested", + .detail = try std.fmt.allocPrint(arena, "harvested set but harvested_date missing (annotation will not render): {s}", .{ + try joinCapped(arena, undated.items, 6), + }), + }; + } + return .{ + .status = .warn, + .label = "accounts.srf harvested", + .detail = try std.fmt.allocPrint(arena, "harvested_date is in the future (annotation will not render): {s}", .{ + try joinCapped(arena, future.items, 6), + }), + }; +} + /// Build the per-key capability checks from a resolved `Config`. Pure /// over `Config` (no I/O), so every branch is unit-testable by /// constructing a `Config` literal. Present keys -> OK with the @@ -404,8 +472,10 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { const lot_accts = try uniqueAccounts(arena, all_lots.items); const known = try accountNames(arena, am); try checks.append(arena, try coverageCheck(arena, "accounts.srf coverage", lot_accts, known, "accounts in portfolio missing from accounts.srf")); + try checks.append(arena, try harvestedFieldChecks(arena, am, ctx.today)); } else { try checks.append(arena, .{ .status = .info, .label = "accounts.srf coverage", .detail = "skipped (accounts.srf not loaded)" }); + try checks.append(arena, .{ .status = .info, .label = "accounts.srf harvested", .detail = "skipped (accounts.srf not loaded)" }); } // Metadata (classification) coverage for classifiable holdings. @@ -770,6 +840,90 @@ test "coverageCheck: dedupes repeated missing names" { try testing.expectEqual(@as(usize, 1), std.mem.count(u8, c.detail, "Sample HSA")); } +test "harvestedFieldChecks: no account declares harvested -> info" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + const c = try harvestedFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.info, c.status); +} + +test "harvestedFieldChecks: dated figures are ok and counted" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2026-06-24 + \\account::Sample Trust,tax_type::taxable,harvested:num:-8000,harvested_date::2025-11-01 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + const c = try harvestedFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.ok, c.status); + try testing.expect(std.mem.indexOf(u8, c.detail, "2 account(s)") != null); +} + +test "harvestedFieldChecks: a stale harvested_date is not a warning" { + // Retiring the annotation after 12 months is designed behavior, not + // an error - doctor must stay quiet about it. + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2020-01-01 + ); + defer am.deinit(); + const c = try harvestedFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.ok, c.status); +} + +test "harvestedFieldChecks: harvested without a date warns and names the account" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300 + ); + defer am.deinit(); + const c = try harvestedFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.warn, c.status); + try testing.expect(std.mem.indexOf(u8, c.detail, "harvested_date missing") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Tax Loss") != null); +} + +test "harvestedFieldChecks: future harvested_date warns and names the account" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2027-06-24 + ); + defer am.deinit(); + const c = try harvestedFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.warn, c.status); + try testing.expect(std.mem.indexOf(u8, c.detail, "future") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Tax Loss") != null); +} + +test "harvestedFieldChecks: both problems at once are reported together" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300 + \\account::Sample Trust,tax_type::taxable,harvested:num:8000,harvested_date::2027-06-24 + ); + defer am.deinit(); + const c = try harvestedFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.warn, c.status); + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Tax Loss") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Trust") != null); +} + test "capabilityChecks: present keys are ok, absent keys are info (never warn)" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); diff --git a/src/format.zig b/src/format.zig index cec3b0a..7d5b8ab 100644 --- a/src/format.zig +++ b/src/format.zig @@ -218,8 +218,29 @@ pub fn fmtTimeAgo(buf: []u8, before_s: i64, after_s: i64) []const u8 { return std.fmt.bufPrint(buf, "{d}d ago", .{@as(u64, @intCast(@divFloor(delta, std.time.s_per_day)))}) catch "?"; } +/// Knobs for `fmtLargeNumOpts`. +pub const LargeNumOpts = struct { + /// Emit a whole-thousands `k` suffix for values in + /// [1_000, 1_000_000). OFF by default, deliberately: the + /// chart-axis glyph set (`charts/text.zig` `glyphFor`) has + /// bitmaps for T/B/M but not `k`, so an axis label of "$45k" + /// would stamp as "$45 ". Sub-million axis labels already use + /// comma'd whole dollars, so they don't want this either. + /// + /// The `k` tier renders whole thousands ("45k") rather than the + /// one decimal place T/B/M use ("1.5B"). At this magnitude a + /// decimal is noise - the callers that want `k` are compact + /// annotations where every column counts. + thousands: bool = false, +}; + /// Format large numbers with T/B/M suffixes (e.g. "1.5B", "45.6M"). pub fn fmtLargeNum(val: f64) [15]u8 { + return fmtLargeNumOpts(val, .{}); +} + +/// `fmtLargeNum` with tier control. See `LargeNumOpts`. +pub fn fmtLargeNumOpts(val: f64, opts: LargeNumOpts) [15]u8 { var result: [15]u8 = @splat(' '); // bufPrint can only fail with NoSpaceLeft, which is impossible // here: a 15-byte buffer comfortably holds any "{d:.1}" value @@ -230,12 +251,44 @@ pub fn fmtLargeNum(val: f64) [15]u8 { _ = std.fmt.bufPrint(&result, "{d:.1}B", .{val / 1_000_000_000}) catch |err| std.debug.panic("fmtLargeNum buffer too small: {t}", .{err}); } else if (val >= 1_000_000) { _ = std.fmt.bufPrint(&result, "{d:.1}M", .{val / 1_000_000}) catch |err| std.debug.panic("fmtLargeNum buffer too small: {t}", .{err}); + } else if (opts.thousands and val >= 1_000) { + _ = std.fmt.bufPrint(&result, "{d:.0}k", .{val / 1_000}) catch |err| std.debug.panic("fmtLargeNum buffer too small: {t}", .{err}); } else { _ = std.fmt.bufPrint(&result, "{d:.0}", .{val}) catch |err| std.debug.panic("fmtLargeNum buffer too small: {t}", .{err}); } return result; } +/// Widest string `fmtHarvestAnnotation` can produce: "(999.9M 12/31)". +/// Callers sizing a stack buffer should use this. +pub const harvest_annotation_max_len = "(999.9M 12/31)".len; + +/// Compact tax-loss-harvested annotation for an account row: +/// `"(45k 6/24)"`. Returns `""` (nothing to render) when `amount` or +/// `harvest_date` is unset, when the date is in the future relative to +/// `as_of`, or when it is more than 12 months old. +/// +/// The 12-month gate is what makes a bare `M/D` unambiguous: within a +/// trailing-year window with future dates excluded, each (month, day) +/// pair occurs at most once, so no year is needed. Harvest data older +/// than a year is worthless for this purpose anyway, so there is +/// deliberately no month/year fallback for stale entries - they just +/// stop rendering. +/// +/// `amount` is sign-insensitive (`accounts.srf` accepts either +/// `harvested:num:45300` or `harvested:num:-45300`); the parens carry +/// the "this is a loss" affordance, borrowing the accounting +/// convention. `buf` must be at least `harvest_annotation_max_len`. +pub fn fmtHarvestAnnotation(buf: []u8, amount: ?f64, harvest_date: ?Date, as_of: Date) []const u8 { + const amt = amount orelse return ""; + const on = harvest_date orelse return ""; + if (as_of.lessThan(on)) return ""; // future-dated + if (on.lessThan(as_of.subtractYears(1))) return ""; // stale + const mag = fmtLargeNumOpts(@abs(amt), .{ .thousands = true }); + const trimmed = std.mem.trimEnd(u8, &mag, " "); + return std.fmt.bufPrint(buf, "({s} {d}/{d})", .{ trimmed, on.month(), on.day() }) catch ""; +} + // ── Display-width helpers ──────────────────────────────────── /// Count the number of terminal display columns occupied by a @@ -963,6 +1016,96 @@ test "fmtLargeNum" { try std.testing.expect(std.mem.startsWith(u8, &tril, "2.3T")); } +test "fmtLargeNumOpts: thousands off leaves every tier byte-identical to fmtLargeNum" { + // Guards the chart-axis callers (charts/axis.zig, quote_tab, + // commands/etf) against a behavior change from the new option. + const vals = [_]f64{ 0, 999, 1_000, 12_345, 999_999, 45_600_000, 1_500_000_000, 2_300_000_000_000 }; + for (vals) |v| { + const default = fmtLargeNum(v); + const explicit = fmtLargeNumOpts(v, .{ .thousands = false }); + try std.testing.expectEqualStrings(&default, &explicit); + } +} + +test "fmtLargeNumOpts: thousands on adds a whole-thousands k tier" { + const k = fmtLargeNumOpts(45_300.0, .{ .thousands = true }); + try std.testing.expectEqualStrings("45k", std.mem.trimEnd(u8, &k, " ")); + // Rounds to nearest whole thousand. + const up = fmtLargeNumOpts(45_800.0, .{ .thousands = true }); + try std.testing.expectEqualStrings("46k", std.mem.trimEnd(u8, &up, " ")); + // Boundaries: 999 stays bare, 1000 becomes 1k, 1M defers to the M tier. + const under = fmtLargeNumOpts(999.0, .{ .thousands = true }); + try std.testing.expectEqualStrings("999", std.mem.trimEnd(u8, &under, " ")); + const at = fmtLargeNumOpts(1_000.0, .{ .thousands = true }); + try std.testing.expectEqualStrings("1k", std.mem.trimEnd(u8, &at, " ")); + const mil = fmtLargeNumOpts(1_000_000.0, .{ .thousands = true }); + try std.testing.expectEqualStrings("1.0M", std.mem.trimEnd(u8, &mil, " ")); +} + +test "fmtHarvestAnnotation: renders compact amount and month/day" { + var buf: [harvest_annotation_max_len]u8 = undefined; + const as_of = Date.fromYmd(2026, 7, 25); + try std.testing.expectEqualStrings( + "(45k 6/24)", + fmtHarvestAnnotation(&buf, 45_300, Date.fromYmd(2026, 6, 24), as_of), + ); + // Millions tier keeps one decimal. + try std.testing.expectEqualStrings( + "(1.2M 12/31)", + fmtHarvestAnnotation(&buf, 1_240_000, Date.fromYmd(2025, 12, 31), as_of), + ); + // Sub-thousand renders bare. + try std.testing.expectEqualStrings( + "(750 7/1)", + fmtHarvestAnnotation(&buf, 750, Date.fromYmd(2026, 7, 1), as_of), + ); +} + +test "fmtHarvestAnnotation: sign-insensitive on the amount" { + var buf: [harvest_annotation_max_len]u8 = undefined; + const as_of = Date.fromYmd(2026, 7, 25); + const on = Date.fromYmd(2026, 6, 24); + const positive = fmtHarvestAnnotation(&buf, 45_300, on, as_of); + var buf2: [harvest_annotation_max_len]u8 = undefined; + const negative = fmtHarvestAnnotation(&buf2, -45_300, on, as_of); + try std.testing.expectEqualStrings(positive, negative); + try std.testing.expectEqualStrings("(45k 6/24)", negative); +} + +test "fmtHarvestAnnotation: suppressed when either input is unset" { + var buf: [harvest_annotation_max_len]u8 = undefined; + const as_of = Date.fromYmd(2026, 7, 25); + try std.testing.expectEqualStrings("", fmtHarvestAnnotation(&buf, null, Date.fromYmd(2026, 6, 24), as_of)); + try std.testing.expectEqualStrings("", fmtHarvestAnnotation(&buf, 45_300, null, as_of)); + try std.testing.expectEqualStrings("", fmtHarvestAnnotation(&buf, null, null, as_of)); +} + +test "fmtHarvestAnnotation: 12-month window boundaries" { + var buf: [harvest_annotation_max_len]u8 = undefined; + const as_of = Date.fromYmd(2026, 7, 25); + + // Same day as `as_of` - in window. + try std.testing.expect(fmtHarvestAnnotation(&buf, 45_300, as_of, as_of).len > 0); + // 11 months old - in window. + try std.testing.expect(fmtHarvestAnnotation(&buf, 45_300, Date.fromYmd(2025, 8, 25), as_of).len > 0); + // Exactly 12 months old - in window (inclusive lower bound). + try std.testing.expect(fmtHarvestAnnotation(&buf, 45_300, Date.fromYmd(2025, 7, 25), as_of).len > 0); + // One day past 12 months - suppressed. + try std.testing.expectEqualStrings("", fmtHarvestAnnotation(&buf, 45_300, Date.fromYmd(2025, 7, 24), as_of)); + // 13 months old - suppressed. + try std.testing.expectEqualStrings("", fmtHarvestAnnotation(&buf, 45_300, Date.fromYmd(2025, 6, 25), as_of)); + // Future-dated - suppressed (a typo, not data). + try std.testing.expectEqualStrings("", fmtHarvestAnnotation(&buf, 45_300, Date.fromYmd(2026, 7, 26), as_of)); +} + +test "fmtHarvestAnnotation: widest output fits harvest_annotation_max_len" { + var buf: [harvest_annotation_max_len]u8 = undefined; + const as_of = Date.fromYmd(2026, 1, 1); + const s = fmtHarvestAnnotation(&buf, 999_900_000, Date.fromYmd(2025, 12, 31), as_of); + try std.testing.expectEqualStrings("(999.9M 12/31)", s); + try std.testing.expectEqual(harvest_annotation_max_len, s.len); +} + test "fmtCashHeader" { var buf: [128]u8 = undefined; const header = fmtCashHeader(&buf); diff --git a/src/tui/analysis_tab.zig b/src/tui/analysis_tab.zig index 4cf96b5..a02bb42 100644 --- a/src/tui/analysis_tab.zig +++ b/src/tui/analysis_tab.zig @@ -385,8 +385,17 @@ pub fn fmtBreakdownLine(arena: std.mem.Allocator, item: zfin.analysis.BreakdownI const padded_label = try arena.alloc(u8, trimmed.len + pad_cols); @memcpy(padded_label[0..trimmed.len], trimmed); if (pad_cols > 0) @memset(padded_label[trimmed.len..], ' '); - return std.fmt.allocPrint(arena, " {s} {s} {d:>5.1}% {f}", .{ - padded_label, bar, pct, Money.from(item.value), + // Optional trailing annotation (the "By Account" breakdown's + // per-account harvested figure). Appended after the value so it + // can't disturb the label padding or the caller's bar-fill span + // offsets. Unstyled - the parens are the affordance, and this line's + // single `alt_*` span is already spent on the bar. + const annotation: []const u8 = if (item.annotation) |*ann| + try std.fmt.allocPrint(arena, " {s}", .{ann.slice()}) + else + ""; + return std.fmt.allocPrint(arena, " {s} {s} {d:>5.1}% {f}{s}", .{ + padded_label, bar, pct, Money.from(item.value), annotation, }); } @@ -442,6 +451,30 @@ test "fmtBreakdownLine formats correctly" { try testing.expect(std.mem.indexOf(u8, line, "US Stock") != null); try testing.expect(std.mem.indexOf(u8, line, "65.0%") != null); try testing.expect(std.mem.indexOf(u8, line, "$130,000") != null); + // No annotation set -> nothing appended. + try testing.expect(std.mem.endsWith(u8, line, "$130,000.00")); +} + +test "fmtBreakdownLine: annotation appends after the value without shifting the bar" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const plain = zfin.analysis.BreakdownItem{ + .label = "Sample Tax Loss", + .weight = 0.4, + .value = 412_300, + }; + var annotated = plain; + annotated.annotation = zfin.analysis.Annotation.from("(45k 6/24)"); + + const bare = try fmtBreakdownLine(arena, plain, 10, 12); + const with = try fmtBreakdownLine(arena, annotated, 10, 12); + + // The annotated line is the bare line plus the suffix - so every + // column the caller's bar-fill span references is unmoved. + try testing.expectEqualStrings(bare, with[0..bare.len]); + try testing.expectEqualStrings(" (45k 6/24)", with[bare.len..]); } test "renderAnalysisLines with data" { diff --git a/src/tui/portfolio_tab.zig b/src/tui/portfolio_tab.zig index 9ca67e1..dd4a88a 100644 --- a/src/tui/portfolio_tab.zig +++ b/src/tui/portfolio_tab.zig @@ -234,6 +234,11 @@ pub const State = struct { /// Auto-assigned shortcut-key per account, parallel to /// `account_list`. Used by the account picker modal. account_shortcut_keys: std.ArrayList(u8) = .empty, + /// Pre-rendered harvested annotation per account, parallel to + /// `account_list`. Null when the account has no fresh + /// `harvested` entry in `accounts.srf`. Value type, so nothing + /// here needs freeing - see `analysis.Annotation`. + account_harvested: std.ArrayList(?zfin.analysis.Annotation) = .empty, // ── Account picker / search modal ── // // The portfolio tab owns picker state in full: the cursor, @@ -346,6 +351,7 @@ pub const tab = struct { state.account_list.deinit(app.allocator); state.account_numbers.deinit(app.allocator); state.account_shortcut_keys.deinit(app.allocator); + state.account_harvested.deinit(app.allocator); state.account_search_matches.deinit(app.allocator); state.* = .{}; } @@ -1343,13 +1349,36 @@ pub fn setAccountFilter(state: *State, app: *App, name: ?[]const u8) void { } } +/// Look up the pre-rendered harvested annotation for `account` in the +/// parallel account arrays built by `buildAccountList`. +/// +/// Reads the cached annotation rather than re-consulting +/// `app.portfolio.accountMap()`, so the draw path never blocks on the +/// account-map worker. Tolerates the arrays being shorter than each +/// other (they're appended independently and an OOM mid-append can +/// leave them ragged). +fn lookupHarvestAnnotation( + account_list: []const []const u8, + account_harvested: []const ?zfin.analysis.Annotation, + account: []const u8, +) ?zfin.analysis.Annotation { + for (account_list, 0..) |name, i| { + if (!std.mem.eql(u8, name, account)) continue; + if (i >= account_harvested.len) return null; + return account_harvested[i]; + } + return null; +} + /// Build the ordered list of distinct account names from portfolio lots. /// Order: accounts.srf file order first, then any remaining accounts alphabetically. -/// Also assigns shortcut keys and loads account numbers from accounts.srf. +/// Also assigns shortcut keys and loads account numbers plus harvested +/// annotations from accounts.srf. pub fn buildAccountList(state: *State, app: *App) void { state.account_list.clearRetainingCapacity(); state.account_numbers.clearRetainingCapacity(); state.account_shortcut_keys.clearRetainingCapacity(); + state.account_harvested.clearRetainingCapacity(); const pf = app.portfolio.file orelse return; @@ -1377,6 +1406,11 @@ pub fn buildAccountList(state: *State, app: *App) void { if (seen.contains(entry.account)) { state.account_list.append(app.allocator, entry.account) catch continue; state.account_numbers.append(app.allocator, entry.account_number) catch continue; + var ann_buf: [fmt.harvest_annotation_max_len]u8 = undefined; + const ann = zfin.analysis.Annotation.from( + fmt.fmtHarvestAnnotation(&ann_buf, entry.harvested, entry.harvested_date, app.today), + ); + state.account_harvested.append(app.allocator, ann) catch continue; } } } @@ -1405,6 +1439,8 @@ pub fn buildAccountList(state: *State, app: *App) void { for (extras.items) |acct| { state.account_list.append(app.allocator, acct) catch continue; state.account_numbers.append(app.allocator, null) catch continue; + // Not in accounts.srf, so by definition no harvested entry. + state.account_harvested.append(app.allocator, null) catch continue; } // Assign shortcut keys: 1-9, 0, then b-z (skipping conflict keys) @@ -1615,8 +1651,13 @@ pub fn drawContent(state: *State, app: *App, arena: std.mem.Allocator, buf: []va const filtered_gl = filtered_value - filtered_cost; const filtered_return = if (filtered_cost > 0) (filtered_gl / filtered_cost) else @as(f64, 0); - // Account name line - const acct_text = try std.fmt.allocPrint(arena, " Account: {s}", .{af}); + // Account name line, with the harvested annotation when the + // account carries one. + const harvest = lookupHarvestAnnotation(state.account_list.items, state.account_harvested.items, af); + const acct_text = if (harvest) |*h| + try std.fmt.allocPrint(arena, " Account: {s} {s}", .{ af, h.slice() }) + else + try std.fmt.allocPrint(arena, " Account: {s}", .{af}); try lines.append(arena, .{ .text = acct_text, .style = th.headerStyle() }); const gl_abs = if (filtered_gl >= 0) filtered_gl else -filtered_gl; @@ -2279,6 +2320,41 @@ pub fn reloadPortfolioFile(state: *State, app: *App) void { /// Used for mouse click hit-testing. pub const account_picker_header_lines: usize = 3; +/// Build one account-picker row's text: selection marker, a 3-column +/// shortcut-key cell, the account name, then optional trailing +/// metadata - the `accounts.srf` account number and the harvested +/// annotation. +/// +/// The account number renders bare (`- 1234`) rather than +/// parenthesized: the harvested annotation that may follow it is +/// itself parenthesized, and two adjacent paren groups read as noise. +/// Search still matches on the number - `updateAccountSearchMatches` +/// consults `state.account_numbers` directly, not this display text. +fn formatAccountPickerRow( + arena: std.mem.Allocator, + marker: []const u8, + shortcut: u8, + label: []const u8, + acct_num: ?[]const u8, + harvested: ?zfin.analysis.Annotation, +) ![]const u8 { + // 3 columns either way, so names stay aligned whether or not the + // account got a shortcut key. + const key: []const u8 = if (shortcut != 0) + try std.fmt.allocPrint(arena, "{c}: ", .{shortcut}) + else + " "; + const num_suffix: []const u8 = if (acct_num) |n| + try std.fmt.allocPrint(arena, " - {s}", .{n}) + else + ""; + const harvest_suffix: []const u8 = if (harvested) |*h| + try std.fmt.allocPrint(arena, " {s}", .{h.slice()}) + else + ""; + return std.fmt.allocPrint(arena, "{s}{s}{s}{s}{s}", .{ marker, key, label, num_suffix, harvest_suffix }); +} + /// Draw the account picker overlay (replaces portfolio content). pub fn drawAccountPicker(state: *State, app: *App, arena: std.mem.Allocator, buf: []vaxis.Cell, width: u16, height: u16) !void { const th = app.theme; @@ -2319,16 +2395,9 @@ pub fn drawAccountPicker(state: *State, app: *App, arena: std.mem.Allocator, buf const label = state.account_list.items[acct_idx]; const shortcut: u8 = if (acct_idx < state.account_shortcut_keys.items.len) state.account_shortcut_keys.items[acct_idx] else 0; const acct_num: ?[]const u8 = if (acct_idx < state.account_numbers.items.len) state.account_numbers.items[acct_idx] else null; + const harvested: ?zfin.analysis.Annotation = if (acct_idx < state.account_harvested.items.len) state.account_harvested.items[acct_idx] else null; - const text = if (acct_num) |num| - (if (shortcut != 0) - try std.fmt.allocPrint(arena, "{s}{c}: {s} ({s})", .{ marker, shortcut, label, num }) - else - try std.fmt.allocPrint(arena, "{s} {s} ({s})", .{ marker, label, num })) - else if (shortcut != 0) - try std.fmt.allocPrint(arena, "{s}{c}: {s}", .{ marker, shortcut, label }) - else - try std.fmt.allocPrint(arena, "{s} {s}", .{ marker, label }); + const text = try formatAccountPickerRow(arena, marker, shortcut, label, acct_num, harvested); var style = if (is_selected) th.selectStyle() else th.contentStyle(); if (is_searching and state.account_search_len > 0) { @@ -2793,6 +2862,70 @@ test "statusSuffix: formats the active account filter, null when unset" { try testing.expect(tab.formatAccountSuffix(arena, null) == null); } +test "formatAccountPickerRow: shortcut, bare account number, harvested annotation" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + const ann = zfin.analysis.Annotation.from("(45k 6/24)"); + + // Everything present. The account number is deliberately NOT + // parenthesized so it doesn't collide with the annotation. + try testing.expectEqualStrings( + " > 1: Sample Tax Loss - 1234 (45k 6/24)", + try formatAccountPickerRow(arena, " > ", '1', "Sample Tax Loss", "1234", ann), + ); + // No annotation. + try testing.expectEqualStrings( + " 1: Sample Brokerage - 5678", + try formatAccountPickerRow(arena, " ", '1', "Sample Brokerage", "5678", null), + ); + // No account number, annotation present. + try testing.expectEqualStrings( + " 1: Sample Tax Loss (45k 6/24)", + try formatAccountPickerRow(arena, " ", '1', "Sample Tax Loss", null, ann), + ); + // Neither. + try testing.expectEqualStrings( + " 1: Sample Trust", + try formatAccountPickerRow(arena, " ", '1', "Sample Trust", null, null), + ); +} + +test "formatAccountPickerRow: no shortcut still occupies 3 columns so names align" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const with_key = try formatAccountPickerRow(arena, " ", '1', "Sample Trust", null, null); + const without = try formatAccountPickerRow(arena, " ", 0, "Sample Trust", null, null); + try testing.expectEqualStrings(" Sample Trust", without); + try testing.expectEqual(with_key.len, without.len); + try testing.expectEqual( + std.mem.indexOf(u8, with_key, "Sample Trust").?, + std.mem.indexOf(u8, without, "Sample Trust").?, + ); +} + +test "lookupHarvestAnnotation: finds by name, null when absent or ragged" { + const names = [_][]const u8{ "Sample IRA", "Sample Tax Loss", "Sample Trust" }; + const anns = [_]?zfin.analysis.Annotation{ + null, + zfin.analysis.Annotation.from("(45k 6/24)"), + null, + }; + + const hit = lookupHarvestAnnotation(&names, &anns, "Sample Tax Loss").?; + try testing.expectEqualStrings("(45k 6/24)", hit.slice()); + // Present in the list but with no annotation. + try testing.expect(lookupHarvestAnnotation(&names, &anns, "Sample IRA") == null); + // Not in the list at all. + try testing.expect(lookupHarvestAnnotation(&names, &anns, "Nope") == null); + // Ragged parallel arrays (OOM mid-append) must not read out of bounds. + try testing.expect(lookupHarvestAnnotation(&names, anns[0..1], "Sample Tax Loss") == null); + // Empty. + try testing.expect(lookupHarvestAnnotation(&.{}, &.{}, "Sample IRA") == null); +} + test "ensureCursorVisible: cursor above viewport scrolls up" { var state: State = .{ .cursor = 5, .header_lines = 2 }; var scroll: usize = 20;