From 4f1ca3d34dca95ed08c07e272ebb6a1258beca94 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sat, 1 Aug 2026 15:32:33 -0700 Subject: [PATCH] add ability to handle opaque 401(k)s with new tax_mix fields --- AGENTS.md | 57 +- docs/explanation/faq-troubleshooting.md | 17 + docs/guides/set-up-accounts.md | 15 +- docs/reference/cli/analysis.md | 6 + docs/reference/cli/audit.md | 8 + docs/reference/cli/doctor.md | 5 +- docs/reference/config/accounts-srf.md | 106 ++- examples/pre-retirement-both/accounts.srf | 6 +- src/analytics/analysis.zig | 804 +++++++++++++++++++++- src/commands/audit/hygiene.zig | 457 ++++++++++-- src/commands/doctor.zig | 226 ++++++ src/views/review.zig | 83 ++- 12 files changed, 1654 insertions(+), 136 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e4b4575..5beb6ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -620,35 +620,44 @@ commit so the gain is locked in. ```bash zig build coverage -# Open coverage/index.html in a browser, or: -ls coverage/ # per-file HTML reports +covq diff # what did this change leave uncovered? +open coverage/index.html # or browse the HTML report ``` -Each file's report shows red lines (uncovered) and green lines -(covered). For a quick numeric breakdown by file, the kcov JSON -output under `coverage/kcov-merged/coverage.json` is greppable. +The authoritative machine-readable outputs are all under +`coverage/zfin/`: `coverage.json` (per-file totals, what the threshold +gate reads), `sonarqube.xml` and `cobertura.xml` (per-line). Note that +`coverage/kcov-merged/` is empty - there is only one test binary, so +kcov never merges anything. -**Two traps if you script against the coverage output.** Both have -already produced a confidently-wrong "fully covered" claim: +**Use `covq` for per-file / per-line / per-function queries.** It's on +PATH and understands the `sonarqube.xml` / `cobertura.xml` that kcov +writes alongside the HTML: -1. **Per-line data lives in the `.js` files, not the `.html`.** Each - `coverage//..html` has only a handful of - `class="lineNum"` nodes; the real per-line records are in the - sibling `.js` as - `{"lineNum":" 42","line":"...","class":"lineCov|lineNoCov|linePartCov"}`. - A scraper pointed at the HTML matches **zero** lines, so - "0 uncovered" silently means "0 tracked." Always assert that the - tracked-line count is non-zero before trusting a coverage verdict, - and sanity-check the scraper against a line you know is uncovered. -2. **`coverage/` accumulates stale per-binary directories across - runs.** Old reports for a file you just edited stay on disk with the - previous source text. Filter to reports whose embedded `"line"` - content matches the current file (grep for a string you just added) - before unioning them. +```bash +covq diff # coverage of what you just changed +covq file src/analytics/analysis.zig # every instrumented line +covq fn src/format.zig fmtHarvestAnnotation # one function +``` -Note that per-file numbers are a union across binaries: the same source -file appears in several `coverage//` directories, and a line -covered by any of them counts. +Do NOT scrape the HTML report by hand. See +`@~/.config/agents/rules/coverage.md` for why; the short version is that +kcov's `.html` files carry almost no per-line data (it's in the sibling +`.js`), so a scraper pointed at them matches **zero** lines and reports +"0 uncovered", which reads as "fully covered." That has already +produced a confidently-wrong claim in this repo. + +**There is exactly one test binary, and exactly one authoritative +report.** `build.zig` registers a single module (`cov.addModule(cov_mod, +"zfin")`), and `build/Coverage.zig` reads +`coverage/zfin/coverage.json` - where `coverage/zfin` is a **symlink to +the current run**. The sibling `coverage/zfin./` directories are +previous runs that nothing reads. + +Never union across those directories. They hold results for older +source, so merging them can make a currently-uncovered line look +covered. Read `coverage/zfin/` and nothing else. (They also accumulate +without bound - the directory has reached multiple GB.) **Common reasons coverage looks lower than expected:** diff --git a/docs/explanation/faq-troubleshooting.md b/docs/explanation/faq-troubleshooting.md index b65b30e..cb18780 100644 --- a/docs/explanation/faq-troubleshooting.md +++ b/docs/explanation/faq-troubleshooting.md @@ -34,6 +34,23 @@ or the `account::` name doesn't match the lot's `account::` exactly (names must match character-for-character). See [Map your accounts](../guides/set-up-accounts.md). +**My 401(k) lumps Roth and pre-tax money into one balance, so the +tax-type breakdown is wrong.** +Declare the split instead of picking a side. Add a percentage carve-out +to the account and `tax_type` keeps the remainder: + +```srf +account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-08-01 +``` + +The account then contributes to both the Traditional and Roth rows, +weighted by value. Don't split the account in two in `portfolio.srf` to +work around this -- two accounts can't share one `account_number` +(breaking audit reconciliation), you'd be hand-splitting every lot into +fractional shares on each contribution, and it permanently forks the +account's snapshot history. See +[Mixed tax treatment](../reference/config/accounts-srf.md#mixed-tax-treatment). + **A stock or ETF shows "no earnings data."** ETFs, mutual funds, CUSIPs, and some dual-class shares (e.g. BRK.B) return no earnings on FMP's free tier. This is an expected limitation, diff --git a/docs/guides/set-up-accounts.md b/docs/guides/set-up-accounts.md index acfa4ff..0663999 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 -Four optional fields change how analysis and the audit treat an +Five 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* @@ -82,6 +82,19 @@ account -- see the reference for details: - **`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. +- **`tax_mix_roth:num:22.4`** (and the `tax_mix_taxable` / + `tax_mix_traditional` / `tax_mix_hsa` siblings) -- split an account + that holds more than one kind of money. The classic case is a 401(k) + whose single reported balance lumps pre-tax deferrals, the employer + match, and Roth contributions together. The carve-out is what the + account is *not*; `tax_type` keeps the remainder: + + ```srf + account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-08-01 + ``` + + See [Mixed tax treatment](../reference/config/accounts-srf.md#mixed-tax-treatment) + for the rules and the drift caveat. - **`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; diff --git a/docs/reference/cli/analysis.md b/docs/reference/cli/analysis.md index 7a2d24b..a420532 100644 --- a/docs/reference/cli/analysis.md +++ b/docs/reference/cli/analysis.md @@ -40,6 +40,12 @@ ZFIN_HOME=examples/pre-retirement-both zfin analysis The umbrella block's shielded/exposed split can be overridden per account with `shielded:bool:false` in `accounts.srf`. +One account can contribute to several **By Tax Type** rows. An account +with [`tax_mix_*`](../config/accounts-srf.md#mixed-tax-treatment) +carve-outs -- a 401(k) whose single reported balance lumps pre-tax and +Roth money together -- is apportioned across types by weight rather +than landing wholly on its `tax_type`. + ## See also - [Read your portfolio](../../guides/read-your-portfolio.md#analysis-allocation-breakdowns) diff --git a/docs/reference/cli/audit.md b/docs/reference/cli/audit.md index 558a259..e8fbc7a 100644 --- a/docs/reference/cli/audit.md +++ b/docs/reference/cli/audit.md @@ -50,6 +50,14 @@ is omitted entirely for portfolios where no account declares one. Past 12 months the annotation itself retires, and the nag says `no longer displayed` so it can't vanish without explanation. +Accounts declaring +[`tax_mix_*`](../config/accounts-srf.md#mixed-tax-treatment) carve-outs +get the same treatment in a separate **Stale tax-mix figures** section, +on the same 90-day clock. One difference worth knowing: a stale tax mix +**keeps applying**. It feeds real breakdown totals, so retiring it would +silently move your pre-tax vs post-tax picture. This nag is the only +consequence of letting it age. + ## Example (hygiene check) ```bash diff --git a/docs/reference/cli/doctor.md b/docs/reference/cli/doctor.md index 04abd30..fc4f323 100644 --- a/docs/reference/cli/doctor.md +++ b/docs/reference/cli/doctor.md @@ -24,7 +24,10 @@ Four sections, each line tagged `OK` / `INFO` / `WARN` / `FAIL`: 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. + so `doctor` says so. Likewise the + [`tax_mix_*`](../config/accounts-srf.md#mixed-tax-treatment) + carve-outs: a set that doesn't add up silently falls back to the bare + `tax_type`, so `doctor` names the account and the reason. - **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 diff --git a/docs/reference/config/accounts-srf.md b/docs/reference/config/accounts-srf.md index 87f8f6d..1fbb99e 100644 --- a/docs/reference/config/accounts-srf.md +++ b/docs/reference/config/accounts-srf.md @@ -37,6 +37,11 @@ account::Joint taxable,tax_type::taxable,institution::schwab,account_number::JT0 | `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_mix_taxable` | num | No | -- | Percent of this account that is really taxable money despite `tax_type` (see below). Must be > 0. | +| `tax_mix_roth` | num | No | -- | Percent of this account that is really Roth money despite `tax_type` (see below). Must be > 0. | +| `tax_mix_traditional` | num | No | -- | Percent of this account that is really pre-tax money despite `tax_type` (see below). Must be > 0. | +| `tax_mix_hsa` | num | No | -- | Percent of this account that is really HSA money despite `tax_type` (see below). Must be > 0. | +| `tax_mix_date` | date | No | -- | `YYYY-MM-DD` you last refreshed the `tax_mix_*` values. Advisory: the mix applies with or without it. | ## Tax types @@ -47,8 +52,92 @@ account::Joint taxable,tax_type::taxable,institution::schwab,account_number::JT0 | `traditional` | Traditional (Pre-Tax) | | `hsa` | HSA (Triple Tax-Free) | -Any other value is shown as-is. Accounts missing from `accounts.srf` -appear as "Unknown". +These four are the whole list -- an unrecognized value is a hard error, +not a passthrough, so watch for typos and capitals. Accounts missing +from `accounts.srf` entirely appear as "Unknown". + +## Mixed tax treatment + +`tax_type` says an account is one thing. Some accounts are not. + +The case that forces this is an employer 401(k). A single Fidelity or +Vanguard balance routinely lumps together pre-tax deferrals, the +employer match, in-plan Roth, and after-tax/backdoor Roth conversions, +and the plan's own site often will not break out the sources on the +balance page at all. Calling the whole thing `traditional` overstates +your future tax bill; calling it `roth` understates it. Neither answer +helps you decide anything. + +So: declare the parts you know as **carve-out percentages**, and +`tax_type` keeps the rest. + +```srf +#!srfv1 +# 22.4% of this balance is Roth money; the other 77.6% is pre-tax. +account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-08-01 +``` + +That splits the account across two rows in the **By Tax Type** +breakdown of [`zfin analysis`](../cli/analysis.md) and the TUI's +Analysis tab, weighted by value: + +``` + By Tax Type + Traditional (Pre-Tax) ██████████████████▌ 62.1% $1,142,923.81 + Taxable ██████ 20.0% $368,209.99 + Roth (Post-Tax) █████▍ 17.9% $329,916.15 +``` + +You can carve out more than one type at a time -- an after-tax non-Roth +sleeve is taxable-basis money, so +`tax_type::traditional,tax_mix_roth:num:20,tax_mix_taxable:num:5` reads +as 75% pre-tax, 20% Roth, 5% taxable. + +### Rules + +- **Carve-outs are what `tax_type` is *not*.** The percentage left over + belongs to `tax_type`, so the common one-Roth-sleeve case needs + exactly one number. Declaring `tax_mix_traditional` on an account + whose `tax_type` is already `traditional` is rejected -- the primary + type's share is always the residual, and spelling it out invites a + set that quietly sums to 90. +- **They must sum to less than 100.** Exactly 100 would leave `tax_type` + no share at all, which means `tax_type` is wrong rather than + residual. Each value must also be greater than zero (to omit a type, + omit the field). +- **A rejected mix falls back to the bare `tax_type`,** i.e. to the + behavior you'd get with no `tax_mix_*` fields at all. zfin warns on + load and [`zfin doctor`](../cli/doctor.md) reports the specific cause, + because a half-understood declaration silently reshaping your + breakdown would be worse than ignoring it. +- **Omitting all of them changes nothing.** An account with no + `tax_mix_*` fields is 100% its `tax_type`, exactly as before. + +### Why percentages and not dollars + +Because market appreciation applies proportionally across the sleeves. +A percentage stays correct through every market move and only drifts as +you contribute; a dollar figure would be stale by the next close. + +The assumption behind that: all the sleeves hold the same investments. +For a core 401(k) allocation this is nearly always true. If your Roth +sleeve holds materially different funds than your pre-tax sleeve, the +percentage drifts faster than the model implies -- zfin has no way to +know which lot sits in which sleeve, since the plan reports one balance. + +### `tax_mix_date` and staleness + +Contributions shift the mix, so it needs an occasional refresh. +`tax_mix_date` records when you last read the breakdown off the plan's +site, and a flagless [`zfin audit`](../cli/audit.md) lists any account +whose figure is over 90 days old under **Stale tax-mix figures**. + +Unlike [`harvested`](#harvested-and-harvested_date), the date is purely +advisory: **a stale or undated mix keeps applying.** The mix feeds real +breakdown totals, and silently moving your pre-tax vs post-tax picture +because a date aged out would be far worse than showing a slightly +stale split. So zfin nags and never suppresses. `doctor` does warn about +a missing date, since an undated mix is one `audit` can never age. ## `audit_large_lot_threshold` @@ -188,11 +277,22 @@ is "anything not `taxable` is shielded." Override it when that's wrong: IRA protection varies by state and is not modeled automatically; set this explicitly if it matters to you. +Two interactions with [`tax_mix_*`](#mixed-tax-treatment): + +- With no `shielded` override, an account's shielded share is whatever + isn't `taxable` -- so `tax_mix_taxable:num:10` on a pre-tax account + exposes 10% of its value rather than none of it. A Roth/pre-tax mix + changes nothing here, since both are shielded. +- An explicit `shielded` override applies to the **whole** account and + ignores the mix. It's a statement about the account's legal + protection, which a split of its *tax* treatment has no business + overriding. + ## Example (from `examples/pre-retirement-both`) ```srf #!srfv1 -account::Pat 401k,tax_type::traditional,institution::fidelity,account_number::P401 +account::Pat 401k,tax_type::traditional,institution::fidelity,account_number::P401,tax_mix_roth:num:18,tax_mix_date::2024-04-01 account::Pat Roth,tax_type::roth,institution::fidelity,account_number::PROTH account::Sam 401k,tax_type::traditional,institution::vanguard,account_number::S401 account::Sam Roth,tax_type::roth,institution::vanguard,account_number::SROTH diff --git a/examples/pre-retirement-both/accounts.srf b/examples/pre-retirement-both/accounts.srf index 65ac972..4ce5222 100644 --- a/examples/pre-retirement-both/accounts.srf +++ b/examples/pre-retirement-both/accounts.srf @@ -1,8 +1,12 @@ #!srfv1 # Account tax type classification for the pre-retirement example. # tax_type values: taxable, roth, traditional, hsa +# +# Pat's 401(k) reports one lumped balance covering pre-tax deferrals, +# the employer match, and in-plan Roth contributions. The tax_mix_roth +# carve-out splits it across tax types; tax_type keeps the remainder. -account::Pat 401k,tax_type::traditional,institution::fidelity,account_number::P401 +account::Pat 401k,tax_type::traditional,institution::fidelity,account_number::P401,tax_mix_roth:num:18,tax_mix_date::2024-04-01 account::Pat Roth,tax_type::roth,institution::fidelity,account_number::PROTH account::Sam 401k,tax_type::traditional,institution::vanguard,account_number::S401 account::Sam Roth,tax_type::roth,institution::vanguard,account_number::SROTH diff --git a/src/analytics/analysis.zig b/src/analytics/analysis.zig index b6b18ea..9d0aaaa 100644 --- a/src/analytics/analysis.zig +++ b/src/analytics/analysis.zig @@ -80,6 +80,83 @@ pub const TaxType = enum { } }; +/// How one account's value is distributed across tax types. +/// +/// The overwhelmingly common case is a single type at weight 1.0. That +/// is what an account with no `tax_mix_*` carve-outs produces, and it +/// makes every consumer behave exactly as it did before mixed +/// treatment existed. +/// +/// The motivating case for anything else is an employer 401(k) that +/// reports one balance for a sleeve holding pre-tax deferrals, the +/// employer match, in-plan Roth, and after-tax/backdoor Roth money all +/// at once. The plan will not break the balance out per source, so +/// declaring "this account is 22.4% Roth" is the only way to get an +/// honest pre-tax vs post-tax picture out of it. +/// +/// Weights sum to 1.0 (up to float rounding). +pub const TaxMix = struct { + /// Indexed by `@intFromEnum(TaxType)`. + weights: [type_count]f64, + + pub const type_count = @typeInfo(TaxType).@"enum".fields.len; + + /// The whole account is one tax type - the no-carve-outs default. + pub fn single(t: TaxType) TaxMix { + var w = [_]f64{0} ** type_count; + w[@intFromEnum(t)] = 1.0; + return .{ .weights = w }; + } + + pub fn weightOf(self: TaxMix, t: TaxType) f64 { + return self.weights[@intFromEnum(t)]; + } + + /// Share of the account that is not `.taxable`. This is the + /// umbrella-exposure default rule ("anything but taxable is + /// judgment-shielded") generalized to a fraction. + pub fn shieldedWeight(self: TaxMix) f64 { + return 1.0 - self.weightOf(.taxable); + } +}; + +/// Why a declared `tax_mix_*` set was rejected. +/// +/// The parser logs these and `zfin doctor` reports them. In both cases +/// the account falls back to its bare `tax_type`, so a bad declaration +/// degrades to the pre-mixed-treatment behavior rather than silently +/// mis-splitting a balance. +pub const TaxMixProblem = enum { + /// A carve-out is zero or negative. Zero means "omit the field"; + /// negative is meaningless. + non_positive, + /// A carve-out is nan or inf - only reachable by hand-typing it. + not_finite, + /// A carve-out names the account's own `tax_type`. The primary + /// type's share is always the residual, so naming it explicitly is + /// self-contradictory (and invites a set that sums to 90). + redundant_primary, + /// Carve-outs sum to 100 or more, leaving the primary `tax_type` no + /// share of the account at all - so `tax_type` would be a lie. + over_allocated, + + /// Short human-readable cause, for parser warnings and `doctor`. + pub fn label(self: TaxMixProblem) []const u8 { + return switch (self) { + .non_positive => "every tax_mix_* carve-out must be > 0", + .not_finite => "every tax_mix_* carve-out must be a finite number", + .redundant_primary => "tax_mix_* must not name the account's own tax_type", + .over_allocated => "tax_mix_* carve-outs must sum to less than 100", + }; + } +}; + +/// A `TaxMix` plus the reason it fell back, if it did. +pub const CheckedTaxMix = struct { + mix: TaxMix, + problem: ?TaxMixProblem, +}; + /// Account tax type classification entry, parsed from accounts.srf. pub const AccountTaxEntry = struct { account: []const u8, @@ -187,6 +264,107 @@ pub const AccountTaxEntry = struct { /// 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, + /// Percentage of this account's value that is actually `taxable`, + /// `roth`, `traditional`, or `hsa` money despite `tax_type` saying + /// otherwise. See `TaxMix` for why this exists and `taxMixChecked` + /// for the rules. + /// + /// These are *carve-outs*: `tax_type` keeps whatever percentage is + /// left over, so the common single-Roth-sleeve case needs exactly + /// one number. + /// + /// Percentages rather than dollars on purpose: market appreciation + /// applies proportionally across the sleeves, so a percentage stays + /// correct through market moves and only drifts as you contribute. + /// A dollar figure would go stale daily. + tax_mix_taxable: ?f64 = null, + tax_mix_roth: ?f64 = null, + tax_mix_traditional: ?f64 = null, + tax_mix_hsa: ?f64 = null, + /// The "as of" date for the `tax_mix_*` carve-outs - when you last + /// read the source breakdown off the plan's site. + /// + /// Unlike `harvested_date`, this is advisory only: a missing or + /// stale date never suppresses the split, because the mix feeds + /// real breakdown totals rather than a display annotation. Silently + /// changing someone's pre-tax vs post-tax picture because a date + /// aged out would be far worse than showing a slightly stale one. + /// `zfin audit` nags instead; see `audit/hygiene.zig`. + tax_mix_date: ?Date = null, + + /// The carve-out percentage declared for `t`, if any. Exhaustive on + /// purpose: adding a `TaxType` variant should fail to compile here + /// until a matching `tax_mix_*` field exists. + fn carveOut(self: AccountTaxEntry, t: TaxType) ?f64 { + return switch (t) { + .taxable => self.tax_mix_taxable, + .roth => self.tax_mix_roth, + .traditional => self.tax_mix_traditional, + .hsa => self.tax_mix_hsa, + }; + } + + /// Does this account declare any `tax_mix_*` carve-out? True even + /// when the declaration is invalid, because callers that report + /// problems need to know the user tried. + pub fn hasTaxMix(self: AccountTaxEntry) bool { + for (std.enums.values(TaxType)) |t| { + if (self.carveOut(t) != null) return true; + } + return false; + } + + /// Resolve `tax_type` plus any `tax_mix_*` carve-outs into a + /// normalized `TaxMix`, reporting the first problem found. + /// + /// Rules, all of which fall back to `TaxMix.single(tax_type)`: + /// - each carve-out must be finite and strictly positive + /// - no carve-out may name `tax_type` itself + /// - carve-outs must sum to less than 100 + /// + /// Falling back rather than clamping is deliberate: a + /// half-understood declaration should behave like no declaration, + /// not like a guess at what the user meant. + pub fn taxMixChecked(self: AccountTaxEntry) CheckedTaxMix { + var weights = [_]f64{0} ** TaxMix.type_count; + var carved: f64 = 0; + var problem: ?TaxMixProblem = null; + + for (std.enums.values(TaxType)) |t| { + const pct = self.carveOut(t) orelse continue; + if (t == self.tax_type) { + problem = .redundant_primary; + break; + } + if (!std.math.isFinite(pct)) { + problem = .not_finite; + break; + } + if (pct <= 0) { + problem = .non_positive; + break; + } + carved += pct; + weights[@intFromEnum(t)] = pct / 100.0; + } + if (problem == null and carved >= 100) problem = .over_allocated; + + if (problem) |p| return .{ .mix = .single(self.tax_type), .problem = p }; + if (carved == 0) return .{ .mix = .single(self.tax_type), .problem = null }; + + // Residual from the summed stored weights rather than from + // `carved / 100`, so the vector sums as close to 1.0 as f64 + // allows. + var others: f64 = 0; + for (weights) |w| others += w; + weights[@intFromEnum(self.tax_type)] = 1.0 - others; + return .{ .mix = .{ .weights = weights }, .problem = null }; + } + + /// `taxMixChecked` without the diagnostic - the shape consumers want. + pub fn taxMix(self: AccountTaxEntry) TaxMix { + return self.taxMixChecked().mix; + } }; /// Update cadence for manual account maintenance. Parsed from accounts.srf. @@ -232,6 +410,11 @@ pub const AccountMap = struct { } /// Look up the tax type label for a given account name. + /// + /// This is the account's *primary* type. For accounts with + /// `tax_mix_*` carve-outs it is the majority-by-construction + /// residual holder, not the whole story - callers that need to + /// apportion value across types want `taxMixFor` instead. pub fn taxTypeFor(self: AccountMap, account: []const u8) []const u8 { for (self.entries) |e| { if (std.mem.eql(u8, e.account, account)) { @@ -241,6 +424,19 @@ pub const AccountMap = struct { return "Unknown"; } + /// How `account`'s value is distributed across tax types. Null when + /// the account isn't in the map, so each caller picks its own + /// "unclassified" behavior (the tax-type breakdown emits an + /// "Unknown" row; the umbrella calc assumes exposed). + pub fn taxMixFor(self: AccountMap, account: []const u8) ?TaxMix { + for (self.entries) |e| { + if (std.mem.eql(u8, e.account, account)) { + return e.taxMix(); + } + } + return null; + } + /// Find the portfolio account name for a given institution + account number. pub fn findByInstitutionAccount(self: AccountMap, institution: []const u8, account_number: []const u8) ?[]const u8 { for (self.entries) |e| { @@ -332,7 +528,8 @@ pub fn annotateAccountBreakdown(items: []BreakdownItem, account_map: AccountMap, /// 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:`, -/// `harvested:num:` and `harvested_date::`. +/// `harvested:num:`, `harvested_date::`, +/// `tax_mix_:num:` and `tax_mix_date::`. pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !AccountMap { var entries = std.ArrayList(AccountTaxEntry).empty; errdefer { @@ -379,19 +576,29 @@ pub fn parseAccountsFile(allocator: std.mem.Allocator, data: []const u8) !Accoun break :blk null; } else null; - try entries.append(allocator, .{ - .account = try allocator.dupe(u8, entry.account), - .tax_type = entry.tax_type, - .institution = if (entry.institution) |s| try allocator.dupe(u8, s) else null, - .account_number = if (entry.account_number) |s| try allocator.dupe(u8, s) else null, - .update_cadence = entry.update_cadence, - .cash_is_contribution = entry.cash_is_contribution, - .direct_indexing = entry.direct_indexing, - .shielded = entry.shielded, - .audit_large_lot_threshold = lot_threshold, - .harvested = harvested, - .harvested_date = entry.harvested_date, - }); + // A declared tax mix that breaks the rules falls back to the + // bare `tax_type`. Warn so a typo isn't invisible; the raw + // fields are deliberately left in place so `zfin doctor` can + // report the same problem against the file the user edits. + if (entry.taxMixChecked().problem) |p| { + // Silent under `zig build test`: the parser's own tests feed + // invalid mixes on purpose to verify the fallback, and the + // warn spam pollutes test output. + if (!builtin.is_test) + log.warn("accounts.srf: account '{s}': {s}; ignoring the tax mix", .{ entry.account, p.label() }); + } + + // Copy the whole parsed record, then override just the fields + // that need duping or validating. Spelling out every field here + // used to be a footgun: a newly added field with a default would + // silently keep that default instead of the parsed value. + var out = entry; + out.account = try allocator.dupe(u8, entry.account); + out.institution = if (entry.institution) |s| try allocator.dupe(u8, s) else null; + out.account_number = if (entry.account_number) |s| try allocator.dupe(u8, s) else null; + out.audit_large_lot_threshold = lot_threshold; + out.harvested = harvested; + try entries.append(allocator, out); } return .{ @@ -486,9 +693,11 @@ pub const UmbrellaExposure = struct { /// breakdown and the account-tax map. /// /// Shielding decision per account: -/// - If `entry.shielded` is explicitly set, use that. -/// - Else if `entry.tax_type == .taxable`, NOT shielded. -/// - Else (Traditional / Roth / HSA), shielded by default. +/// - If `entry.shielded` is explicitly set, use that for the whole +/// account - a hand-declared legal fact outranks the tax-type proxy. +/// - Else the shielded share is everything that isn't `taxable`, +/// which for an account with `tax_mix_*` carve-outs is a fraction +/// rather than all-or-nothing. /// /// Accounts not in `account_map` default to NOT shielded /// (defensive - if we don't know, assume the value is exposed @@ -505,12 +714,9 @@ pub fn umbrellaExposure( var exposed: f64 = 0; for (account_breakdown) |item| { - const is_shielded = accountIsShielded(item.label, account_map); - if (is_shielded) { - shielded += item.value; - } else { - exposed += item.value; - } + const frac = accountShieldedFraction(item.label, account_map); + shielded += item.value * frac; + exposed += item.value * (1.0 - frac); } const total = shielded + exposed; @@ -524,18 +730,21 @@ pub fn umbrellaExposure( }; } -/// Look up the shielding decision for one account name. -/// Exposed (returns false) when: -/// - Account is not in the map (defensive default). -/// - Explicit `shielded::false` override. -/// - tax_type is `taxable` and no override. -fn accountIsShielded(account: []const u8, account_map: AccountMap) bool { +/// What share of one account's value is judgment-shielded, in [0, 1]. +/// +/// Returns 0 (fully exposed) when the account is not in the map - the +/// defensive default. An explicit `shielded::false` / `shielded::true` +/// override applies to the whole account and short-circuits the +/// tax-type proxy, including any `tax_mix_*` carve-outs: it is a +/// statement about the account's legal protection, which a split of its +/// *tax* treatment has no business overriding. +fn accountShieldedFraction(account: []const u8, account_map: AccountMap) f64 { for (account_map.entries) |e| { if (!std.mem.eql(u8, e.account, account)) continue; - if (e.shielded) |explicit| return explicit; - return e.tax_type != .taxable; + if (e.shielded) |explicit| return if (explicit) 1.0 else 0.0; + return e.taxMix().shieldedWeight(); } - return false; + return 0.0; } // ── Sector -> asset-category bucket ──────────────────────────── @@ -899,13 +1108,26 @@ pub fn analyzePortfolio( try sector_map.put("Options", sprev + opt_total); } - // Tax type breakdown: map each account's total to its tax type + // Tax type breakdown: apportion each account's total across the tax + // types it actually holds (see `TaxMix`). Most accounts are a single + // type at weight 1.0 and land on exactly one row. Accounts absent + // from accounts.srf collapse into a single "Unknown" row. if (account_map) |am| { var acct_iter = acct_map.iterator(); while (acct_iter.next()) |kv| { - const tt = am.taxTypeFor(kv.key_ptr.*); - const prev = tax_map.get(tt) orelse 0; - try tax_map.put(tt, prev + kv.value_ptr.*); + const value = kv.value_ptr.*; + const mix = am.taxMixFor(kv.key_ptr.*) orelse { + const prev = tax_map.get("Unknown") orelse 0; + try tax_map.put("Unknown", prev + value); + continue; + }; + for (std.enums.values(TaxType)) |t| { + const w = mix.weightOf(t); + if (w <= 0) continue; + const label = t.label(); + const prev = tax_map.get(label) orelse 0; + try tax_map.put(label, prev + value * w); + } } } @@ -1505,6 +1727,97 @@ test "umbrellaExposure: realistic mixed portfolio" { try std.testing.expectApproxEqAbs(@as(f64, 0.5833), u.exposed_pct, 0.001); } +test "umbrellaExposure: a taxable carve-out exposes a fraction of one account" { + // A 401(k) with an after-tax non-Roth sleeve: the pre-tax and Roth + // money is ERISA-shielded, but the taxable-basis slice is not, so the + // account splits rather than landing wholly on one side. + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:20,tax_mix_taxable:num:10 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + const accounts = [_]BreakdownItem{ + .{ .label = "Sample 401k", .value = 1_000_000, .weight = 0.8 }, + .{ .label = "Sample Brokerage", .value = 250_000, .weight = 0.2 }, + }; + const u = umbrellaExposure(&accounts, am); + + // 401k: 90% shielded (traditional 70% + roth 20%) = 900,000 + // 10% exposed (taxable carve-out) = 100,000 + // Brokerage: fully exposed = 250,000 + try std.testing.expectApproxEqAbs(@as(f64, 1_250_000), u.total_liquid, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 900_000), u.shielded_value, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 350_000), u.exposed_value, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 0.28), u.exposed_pct, 0.0001); +} + +test "umbrellaExposure: a traditional/roth mix changes nothing" { + // Both halves are shielded, so splitting a 401(k) between pre-tax and + // Roth must leave the umbrella numbers exactly where they were. This + // pins the claim that the common mix is inert here. + var mixed = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4 + ); + defer mixed.deinit(); + var plain = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional + ); + defer plain.deinit(); + + const accounts = [_]BreakdownItem{ + .{ .label = "Sample 401k", .value = 455_000, .weight = 1.0 }, + }; + const a = umbrellaExposure(&accounts, mixed); + const b = umbrellaExposure(&accounts, plain); + try std.testing.expectEqual(b.shielded_value, a.shielded_value); + try std.testing.expectEqual(b.exposed_value, a.exposed_value); + try std.testing.expectEqual(b.exposed_pct, a.exposed_pct); +} + +test "umbrellaExposure: an explicit shielded override outranks the tax mix" { + // `shielded` is a statement about legal protection; a split of the + // account's *tax* treatment has no business overriding it. So the + // override applies wholesale in both directions. + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample DCP,tax_type::traditional,tax_mix_roth:num:30,shielded:bool:false + \\account::Sample Trust,tax_type::taxable,tax_mix_traditional:num:40,shielded:bool:true + ); + defer am.deinit(); + + const accounts = [_]BreakdownItem{ + .{ .label = "Sample DCP", .value = 600_000, .weight = 0.6 }, + .{ .label = "Sample Trust", .value = 400_000, .weight = 0.4 }, + }; + const u = umbrellaExposure(&accounts, am); + + // DCP fully exposed despite 100% of its mix being non-taxable; + // Trust fully shielded despite 60% of its mix being taxable. + try std.testing.expectApproxEqAbs(@as(f64, 400_000), u.shielded_value, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 600_000), u.exposed_value, 1.0); +} + +test "umbrellaExposure: an invalid tax mix falls back without disturbing the split" { + // A rejected carve-out must behave exactly like no carve-out - not + // like a partial or clamped one. + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_taxable:num:100 + ); + defer am.deinit(); + + const accounts = [_]BreakdownItem{ + .{ .label = "Sample 401k", .value = 500_000, .weight = 1.0 }, + }; + const u = umbrellaExposure(&accounts, am); + try std.testing.expectApproxEqAbs(@as(f64, 500_000), u.shielded_value, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 0), u.exposed_value, 1.0); +} + test "TaxType.label" { try std.testing.expectEqualStrings("Taxable", TaxType.taxable.label()); try std.testing.expectEqualStrings("Roth (Post-Tax)", TaxType.roth.label()); @@ -1512,6 +1825,253 @@ test "TaxType.label" { try std.testing.expectEqualStrings("HSA (Triple Tax-Free)", TaxType.hsa.label()); } +// ── TaxMix ─────────────────────────────────────────────────── + +/// Helper: total weight, which every valid mix must drive to 1.0. +fn testMixSum(mix: TaxMix) f64 { + var sum: f64 = 0; + for (mix.weights) |w| sum += w; + return sum; +} + +test "TaxMix.shieldedWeight: excludes only the taxable share" { + // The umbrella default rule, generalized. Roth / traditional / HSA + // are all shielded, so only a taxable slice reduces the weight. + try std.testing.expectEqual(@as(f64, 1.0), TaxMix.single(.traditional).shieldedWeight()); + try std.testing.expectEqual(@as(f64, 1.0), TaxMix.single(.roth).shieldedWeight()); + try std.testing.expectEqual(@as(f64, 1.0), TaxMix.single(.hsa).shieldedWeight()); + try std.testing.expectEqual(@as(f64, 0), TaxMix.single(.taxable).shieldedWeight()); + + const split: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_taxable = 15, + }; + try std.testing.expectApproxEqAbs(@as(f64, 0.85), split.taxMix().shieldedWeight(), 1e-12); +} + +test "AccountTaxEntry.taxMix: no carve-outs -> single mix on tax_type" { + // The backward-compatibility guarantee: an account that says nothing + // about a mix behaves exactly as it did before mixed treatment. + for (std.enums.values(TaxType)) |t| { + const e: AccountTaxEntry = .{ .account = "Sample Account", .tax_type = t }; + const checked = e.taxMixChecked(); + try std.testing.expectEqual(@as(?TaxMixProblem, null), checked.problem); + try std.testing.expectEqual(@as(f64, 1.0), checked.mix.weightOf(t)); + try std.testing.expect(!e.hasTaxMix()); + } +} + +test "AccountTaxEntry.taxMix: one carve-out leaves the residual on tax_type" { + // The motivating case: a 401(k) reporting one balance that is really + // 77.6% pre-tax (deferrals + match) and 22.4% Roth. + const e: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = 22.4, + }; + const checked = e.taxMixChecked(); + try std.testing.expectEqual(@as(?TaxMixProblem, null), checked.problem); + try std.testing.expect(e.hasTaxMix()); + try std.testing.expectApproxEqAbs(@as(f64, 0.224), checked.mix.weightOf(.roth), 1e-12); + try std.testing.expectApproxEqAbs(@as(f64, 0.776), checked.mix.weightOf(.traditional), 1e-12); + try std.testing.expectEqual(@as(f64, 0), checked.mix.weightOf(.taxable)); + try std.testing.expectEqual(@as(f64, 0), checked.mix.weightOf(.hsa)); + try std.testing.expectApproxEqAbs(@as(f64, 1.0), testMixSum(checked.mix), 1e-12); +} + +test "AccountTaxEntry.taxMix: multiple carve-outs across three types" { + // Pre-tax primary, plus an in-plan Roth sleeve and an after-tax + // non-Roth sleeve whose basis is taxable money. + const e: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = 20, + .tax_mix_taxable = 5, + }; + const mix = e.taxMix(); + try std.testing.expectApproxEqAbs(@as(f64, 0.20), mix.weightOf(.roth), 1e-12); + try std.testing.expectApproxEqAbs(@as(f64, 0.05), mix.weightOf(.taxable), 1e-12); + try std.testing.expectApproxEqAbs(@as(f64, 0.75), mix.weightOf(.traditional), 1e-12); + try std.testing.expectApproxEqAbs(@as(f64, 1.0), testMixSum(mix), 1e-12); +} + +test "AccountTaxEntry.taxMix: weights sum to 1.0 for every valid shape" { + const cases = [_]AccountTaxEntry{ + .{ .account = "A", .tax_type = .traditional }, + .{ .account = "B", .tax_type = .traditional, .tax_mix_roth = 22.4 }, + .{ .account = "C", .tax_type = .roth, .tax_mix_traditional = 33.3333 }, + .{ .account = "D", .tax_type = .taxable, .tax_mix_roth = 1, .tax_mix_hsa = 2, .tax_mix_traditional = 3 }, + .{ .account = "E", .tax_type = .hsa, .tax_mix_taxable = 99.9999 }, + // Rejected shapes still produce a normalized single mix. + .{ .account = "F", .tax_type = .traditional, .tax_mix_roth = 150 }, + .{ .account = "G", .tax_type = .traditional, .tax_mix_traditional = 40 }, + }; + for (cases) |e| { + try std.testing.expectApproxEqAbs(@as(f64, 1.0), testMixSum(e.taxMix()), 1e-12); + } +} + +test "AccountTaxEntry.taxMixChecked: non-positive carve-out falls back to tax_type" { + // Zero means "omit the field"; negative is meaningless. Neither is a + // guess worth honoring, so both degrade to the bare tax_type. + for ([_]f64{ 0, -10 }) |bad| { + const e: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = bad, + }; + const checked = e.taxMixChecked(); + try std.testing.expectEqual(@as(?TaxMixProblem, .non_positive), checked.problem); + try std.testing.expectEqual(@as(f64, 1.0), checked.mix.weightOf(.traditional)); + try std.testing.expectEqual(@as(f64, 0), checked.mix.weightOf(.roth)); + // The user still declared something; doctor needs to know. + try std.testing.expect(e.hasTaxMix()); + } +} + +test "AccountTaxEntry.taxMixChecked: non-finite carve-out falls back to tax_type" { + for ([_]f64{ std.math.nan(f64), std.math.inf(f64), -std.math.inf(f64) }) |bad| { + const e: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = bad, + }; + const checked = e.taxMixChecked(); + try std.testing.expectEqual(@as(?TaxMixProblem, .not_finite), checked.problem); + try std.testing.expectEqual(@as(f64, 1.0), checked.mix.weightOf(.traditional)); + } +} + +test "AccountTaxEntry.taxMixChecked: carve-out naming tax_type itself is rejected" { + // The primary's share is always the residual, so naming it is + // self-contradictory - and would invite a set that sums to 90. + const e: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_traditional = 77.6, + .tax_mix_roth = 22.4, + }; + const checked = e.taxMixChecked(); + try std.testing.expectEqual(@as(?TaxMixProblem, .redundant_primary), checked.problem); + try std.testing.expectEqual(@as(f64, 1.0), checked.mix.weightOf(.traditional)); + try std.testing.expectEqual(@as(f64, 0), checked.mix.weightOf(.roth)); +} + +test "AccountTaxEntry.taxMixChecked: carve-outs summing to 100 or more are rejected" { + // Exactly 100 leaves the primary tax_type no share at all, which + // would make tax_type a lie rather than a residual holder. + const exact: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = 60, + .tax_mix_taxable = 40, + }; + try std.testing.expectEqual(@as(?TaxMixProblem, .over_allocated), exact.taxMixChecked().problem); + try std.testing.expectEqual(@as(f64, 1.0), exact.taxMix().weightOf(.traditional)); + + const over: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = 130, + }; + try std.testing.expectEqual(@as(?TaxMixProblem, .over_allocated), over.taxMixChecked().problem); + + // Just under 100 is fine, however little is left for the primary. + const under: AccountTaxEntry = .{ + .account = "Sample 401k", + .tax_type = .traditional, + .tax_mix_roth = 99.5, + }; + try std.testing.expectEqual(@as(?TaxMixProblem, null), under.taxMixChecked().problem); + try std.testing.expectApproxEqAbs(@as(f64, 0.005), under.taxMix().weightOf(.traditional), 1e-12); +} + +test "parseAccountsFile: tax_mix carve-out and tax_mix_date round-trip" { + var am = try parseAccountsFile(std.testing.allocator, + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-08-01 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + try std.testing.expectEqual(@as(usize, 2), am.entries.len); + try std.testing.expectEqual(@as(?f64, 22.4), am.entries[0].tax_mix_roth); + try std.testing.expectEqual(@as(?Date, Date.fromYmd(2026, 8, 1)), am.entries[0].tax_mix_date); + try std.testing.expect(am.entries[0].hasTaxMix()); + + const mix = am.taxMixFor("Sample 401k").?; + try std.testing.expectApproxEqAbs(@as(f64, 0.224), mix.weightOf(.roth), 1e-12); + try std.testing.expectApproxEqAbs(@as(f64, 0.776), mix.weightOf(.traditional), 1e-12); + + // An account with no carve-outs is untouched. + try std.testing.expect(!am.entries[1].hasTaxMix()); + try std.testing.expectEqual(@as(?Date, null), am.entries[1].tax_mix_date); + try std.testing.expectEqual(@as(f64, 1.0), am.taxMixFor("Sample Brokerage").?.weightOf(.taxable)); +} + +test "parseAccountsFile: an invalid tax mix is preserved raw so doctor can report it" { + // The parser deliberately does NOT scrub a rejected carve-out: it + // warns and leaves the field alone, because `zfin doctor` reads back + // through this same parser and has to be able to see what the user + // actually wrote. + var am = try parseAccountsFile(std.testing.allocator, + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:140 + ); + defer am.deinit(); + + try std.testing.expectEqual(@as(usize, 1), am.entries.len); + try std.testing.expectEqual(@as(?f64, 140), am.entries[0].tax_mix_roth); + try std.testing.expectEqual(@as(?TaxMixProblem, .over_allocated), am.entries[0].taxMixChecked().problem); + // ...but the resolved mix falls back to the bare tax_type. + try std.testing.expectEqual(@as(f64, 1.0), am.taxMixFor("Sample 401k").?.weightOf(.traditional)); +} + +test "parseAccountsFile: every non-string field survives the copy-then-override append" { + // Regression guard for the append path in `parseAccountsFile`. It + // copies the parsed record wholesale and overrides only the duped / + // validated fields, precisely so a newly added field can't silently + // keep its default. Assert one value per field. + var am = try parseAccountsFile(std.testing.allocator, + \\#!srfv1 + \\account::Sample Everything,tax_type::roth,institution::fidelity,account_number::1234,update_cadence::quarterly,cash_is_contribution:bool:true,direct_indexing:bool:true,shielded:bool:false,audit_large_lot_threshold:num:50000,harvested:num:-4500,harvested_date::2026-06-24,tax_mix_traditional:num:10,tax_mix_taxable:num:5,tax_mix_hsa:num:1,tax_mix_date::2026-08-01 + ); + defer am.deinit(); + + try std.testing.expectEqual(@as(usize, 1), am.entries.len); + const e = am.entries[0]; + try std.testing.expectEqualStrings("Sample Everything", e.account); + try std.testing.expectEqual(TaxType.roth, e.tax_type); + try std.testing.expectEqualStrings("fidelity", e.institution.?); + try std.testing.expectEqualStrings("1234", e.account_number.?); + try std.testing.expectEqual(UpdateCadence.quarterly, e.update_cadence); + try std.testing.expect(e.cash_is_contribution); + try std.testing.expect(e.direct_indexing); + try std.testing.expectEqual(@as(?bool, false), e.shielded); + try std.testing.expectEqual(@as(?f64, 50_000), e.audit_large_lot_threshold); + try std.testing.expectEqual(@as(?f64, 4500), e.harvested); // sign-normalized + try std.testing.expectEqual(@as(?Date, Date.fromYmd(2026, 6, 24)), e.harvested_date); + try std.testing.expectEqual(@as(?f64, 10), e.tax_mix_traditional); + try std.testing.expectEqual(@as(?f64, 5), e.tax_mix_taxable); + try std.testing.expectEqual(@as(?f64, 1), e.tax_mix_hsa); + try std.testing.expectEqual(@as(?f64, null), e.tax_mix_roth); + try std.testing.expectEqual(@as(?Date, Date.fromYmd(2026, 8, 1)), e.tax_mix_date); +} + +test "taxMixFor: unknown account returns null, distinct from a zero mix" { + var am = try parseAccountsFile(std.testing.allocator, + \\#!srfv1 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + try std.testing.expectEqual(@as(?TaxMix, null), am.taxMixFor("Sample Nowhere")); + try std.testing.expect(am.taxMixFor("Sample Brokerage") != null); + // taxTypeFor keeps its own "Unknown" sentinel for label callers. + try std.testing.expectEqualStrings("Unknown", am.taxTypeFor("Sample Nowhere")); +} + test "mapToSortedBreakdown" { const allocator = std.testing.allocator; var map = std.StringHashMap(f64).init(allocator); @@ -1562,6 +2122,178 @@ test "parseAccountsFile missing fields" { try std.testing.expectEqual(@as(usize, 0), am.entries.len); } +/// Helper: pull one breakdown row's dollar value out by label. +/// Returns null when no row carries that label, which lets a test +/// distinguish "row absent" from "row present with value 0". +fn testBreakdownValue(items: []const BreakdownItem, label: []const u8) ?f64 { + for (items) |it| { + if (std.mem.eql(u8, it.label, label)) return it.value; + } + return null; +} + +/// Helper: a single-symbol portfolio priced at $100/share, with `lots` +/// distributing shares across accounts. Keeps the tax-rollup tests +/// focused on the apportionment rather than on pricing mechanics. +fn testTaxRollup( + account_map: ?AccountMap, + lots: []@import("../models/portfolio.zig").Lot, + total_value: f64, +) !AnalysisResult { + const allocator = std.testing.allocator; + const portfolio = Portfolio{ .lots = lots, .allocator = allocator }; + const allocations = [_]Allocation{.{ + .symbol = "SPY", + .display_symbol = "SPY", + .shares = total_value / 100, + .avg_cost = 100, + .current_price = 100, + .market_value = total_value, + .cost_basis = total_value, + .weight = 1.0, + .unrealized_gain_loss = 0, + .unrealized_return = 0, + .price_ratio = 1.0, + }}; + const cm = ClassificationMap{ .entries = &.{}, .allocator = allocator }; + return analyzePortfolio( + allocator, + &allocations, + cm, + portfolio, + total_value, + account_map, + Date.fromYmd(2026, 8, 1), + ); +} + +test "analyzePortfolio: a mixed account splits its value across tax-type rows" { + // The headline case. One 401(k) reporting a single $400k balance that + // is really 75% pre-tax and 25% Roth, plus a $100k taxable brokerage. + // Before mixed treatment the whole $400k landed on Traditional. + const Lot = @import("../models/portfolio.zig").Lot; + var lots = [_]Lot{ + .{ + .symbol = "SPY", + .shares = 4000, + .open_date = Date.fromYmd(2020, 1, 1), + .open_price = 100, + .account = "Sample 401k", + }, + .{ + .symbol = "SPY", + .shares = 1000, + .open_date = Date.fromYmd(2020, 1, 1), + .open_price = 100, + .account = "Sample Brokerage", + }, + }; + + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:25 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + var result = try testTaxRollup(am, &lots, 500_000); + defer result.deinit(std.testing.allocator); + + try std.testing.expectEqual(@as(usize, 3), result.tax_type.len); + try std.testing.expectApproxEqAbs(@as(f64, 300_000), testBreakdownValue(result.tax_type, "Traditional (Pre-Tax)").?, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 100_000), testBreakdownValue(result.tax_type, "Roth (Post-Tax)").?, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 100_000), testBreakdownValue(result.tax_type, "Taxable").?, 1.0); + + // The split must conserve dollars: the tax-type rows still sum to the + // portfolio total, and the account breakdown is untouched by it. + var tax_sum: f64 = 0; + for (result.tax_type) |it| tax_sum += it.value; + try std.testing.expectApproxEqAbs(@as(f64, 500_000), tax_sum, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 400_000), testBreakdownValue(result.account, "Sample 401k").?, 1.0); + + // Weights are shares of the portfolio total, not of the account. + for (result.tax_type) |it| { + try std.testing.expectApproxEqAbs(it.value / 500_000, it.weight, 1e-9); + } +} + +test "analyzePortfolio: an account with no carve-outs still lands on one row" { + // Backward-compatibility regression: the same fixture without a mix + // must produce exactly two rows with the full account values. + const Lot = @import("../models/portfolio.zig").Lot; + var lots = [_]Lot{ + .{ .symbol = "SPY", .shares = 4000, .open_date = Date.fromYmd(2020, 1, 1), .open_price = 100, .account = "Sample 401k" }, + .{ .symbol = "SPY", .shares = 1000, .open_date = Date.fromYmd(2020, 1, 1), .open_price = 100, .account = "Sample Brokerage" }, + }; + + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + var result = try testTaxRollup(am, &lots, 500_000); + defer result.deinit(std.testing.allocator); + + try std.testing.expectEqual(@as(usize, 2), result.tax_type.len); + try std.testing.expectApproxEqAbs(@as(f64, 400_000), testBreakdownValue(result.tax_type, "Traditional (Pre-Tax)").?, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 100_000), testBreakdownValue(result.tax_type, "Taxable").?, 1.0); + try std.testing.expectEqual(@as(?f64, null), testBreakdownValue(result.tax_type, "Roth (Post-Tax)")); +} + +test "analyzePortfolio: an unmapped account still lands wholly in Unknown" { + // The mix loop replaced `taxTypeFor`, which owned the "Unknown" + // sentinel. Pin that the sentinel survived the move and does not + // get apportioned. + const Lot = @import("../models/portfolio.zig").Lot; + var lots = [_]Lot{ + .{ .symbol = "SPY", .shares = 3000, .open_date = Date.fromYmd(2020, 1, 1), .open_price = 100, .account = "Sample 401k" }, + .{ .symbol = "SPY", .shares = 2000, .open_date = Date.fromYmd(2020, 1, 1), .open_price = 100, .account = "Sample Unlisted" }, + }; + + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:40 + ); + defer am.deinit(); + + var result = try testTaxRollup(am, &lots, 500_000); + defer result.deinit(std.testing.allocator); + + try std.testing.expectEqual(@as(usize, 3), result.tax_type.len); + try std.testing.expectApproxEqAbs(@as(f64, 200_000), testBreakdownValue(result.tax_type, "Unknown").?, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 180_000), testBreakdownValue(result.tax_type, "Traditional (Pre-Tax)").?, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 120_000), testBreakdownValue(result.tax_type, "Roth (Post-Tax)").?, 1.0); +} + +test "analyzePortfolio: two mixed accounts accumulate into shared tax-type rows" { + // Each account contributes to several rows, and several accounts + // contribute to each row. Guards the accumulate-don't-overwrite + // behavior of the nested rollup loop. + const Lot = @import("../models/portfolio.zig").Lot; + var lots = [_]Lot{ + .{ .symbol = "SPY", .shares = 2000, .open_date = Date.fromYmd(2020, 1, 1), .open_price = 100, .account = "Sample 401k A" }, + .{ .symbol = "SPY", .shares = 2000, .open_date = Date.fromYmd(2020, 1, 1), .open_price = 100, .account = "Sample 401k B" }, + }; + + var am = try testParseAccountMap( + \\#!srfv1 + \\account::Sample 401k A,tax_type::traditional,tax_mix_roth:num:25 + \\account::Sample 401k B,tax_type::roth,tax_mix_traditional:num:10 + ); + defer am.deinit(); + + var result = try testTaxRollup(am, &lots, 400_000); + defer result.deinit(std.testing.allocator); + + // A: 200k -> 150k traditional + 50k roth + // B: 200k -> 20k traditional + 180k roth + try std.testing.expectEqual(@as(usize, 2), result.tax_type.len); + try std.testing.expectApproxEqAbs(@as(f64, 170_000), testBreakdownValue(result.tax_type, "Traditional (Pre-Tax)").?, 1.0); + try std.testing.expectApproxEqAbs(@as(f64, 230_000), testBreakdownValue(result.tax_type, "Roth (Post-Tax)").?, 1.0); +} + test "account breakdown applies price_ratio" { const allocator = std.testing.allocator; const Lot = @import("../models/portfolio.zig").Lot; diff --git a/src/commands/audit/hygiene.zig b/src/commands/audit/hygiene.zig index 0c2ca22..0eb21c9 100644 --- a/src/commands/audit/hygiene.zig +++ b/src/commands/audit/hygiene.zig @@ -58,6 +58,20 @@ const max_history_commits_scanned: usize = 500; /// deliberately an independent constant: retuning the account /// reconciliation cadence shouldn't silently move this. const harvested_stale_days: u32 = 90; +/// Age at which `zfin audit` starts nagging that an account's declared +/// `tax_mix_*` carve-outs (see `AccountTaxEntry.tax_mix_taxable`) need a +/// refresh. +/// +/// Same opt-in-by-declaring logic as `harvested_stale_days`, and the +/// same 90 days, but again an independent constant: the two figures +/// drift for unrelated reasons. A tax mix drifts as payroll +/// contributions land in different sleeves, which is roughly quarterly +/// in effect; a harvested total drifts whenever the sleeve churns. +/// +/// Note this nag is the *only* consequence of a stale mix. Unlike the +/// harvested annotation, the mix never stops applying - see +/// `AccountTaxEntry.tax_mix_date`. +const tax_mix_stale_days: u32 = 90; /// Type of a discovered brokerage file. const BrokerFileKind = enum { @@ -436,24 +450,24 @@ fn staleLessThan(_: void, a: StaleManualPrice, b: StaleManualPrice) bool { return std.mem.order(u8, a.symbol, b.symbol) == .lt; } -/// One account whose hand-declared `harvested` figure has gone stale -/// (or was never dated). `account` borrows from the `AccountMap` it was -/// collected from and is valid for that map's lifetime. -const StaleHarvest = struct { +/// One account whose hand-declared, hand-dated figure has gone stale +/// (or was never dated). Shared by the "Stale harvested figures" and +/// "Stale tax-mix figures" sections, which have identical shape: one +/// row per account, worst first. `account` borrows from the +/// `AccountMap` it was collected from and is valid for that map's +/// lifetime. +const StaleDeclared = struct { account: []const u8, - /// Days since `harvested_date`; `null` when the entry declares - /// `harvested` with no `harvested_date` - the worst case, since an - /// undated figure can't be aged and never renders at all. + /// Days since the figure's date; `null` when the figure was declared + /// with no date at all - the worst case, since an undated figure + /// can't be aged. age_days: ?i32, - /// True when the figure no longer renders anywhere. + /// Extra clause appended after the age, or `""` for none. /// - /// Derived by asking `format.fmtHarvestAnnotation` whether it would - /// emit anything, NOT by comparing `age_days` against 365. Those two - /// disagree at leap-year boundaries, because the formatter gates on - /// the calendar-exact `as_of.subtractYears(1)`. Going through the - /// formatter makes the "no longer displayed" message definitionally - /// true - the nag cannot contradict what the user sees. - hidden: bool, + /// Lets the harvested section explain that its annotation has + /// retired without the renderer needing to know what an annotation + /// is. Static strings only. + note: []const u8 = "", }; /// Collect accounts whose `harvested` figure is older than @@ -476,37 +490,87 @@ fn collectStaleHarvested( allocator: std.mem.Allocator, account_map: analysis.AccountMap, as_of: Date, -) !std.ArrayList(StaleHarvest) { - var out = std.ArrayList(StaleHarvest).empty; +) !std.ArrayList(StaleDeclared) { + var out = std.ArrayList(StaleDeclared).empty; errdefer out.deinit(allocator); const threshold: i32 = @intCast(harvested_stale_days); for (account_map.entries) |e| { if (e.harvested == null) continue; + // Whether the figure still renders anywhere is derived by asking + // `format.fmtHarvestAnnotation` whether it would emit anything, + // NOT by comparing the age against 365. Those two disagree at + // leap-year boundaries, because the formatter gates on the + // calendar-exact `as_of.subtractYears(1)`. Going through the + // formatter makes the "no longer displayed" note definitionally + // true - the nag cannot contradict what the user sees. + // // SAFETY: immediately overwritten by fmtHarvestAnnotation below. var ann_buf: [fmt.harvest_annotation_max_len]u8 = undefined; const hidden = fmt.fmtHarvestAnnotation(&ann_buf, e.harvested, e.harvested_date, as_of).len == 0; + const note: []const u8 = if (hidden) " - no longer displayed" else ""; if (e.harvested_date) |on| { const age = as_of.days - on.days; if (age <= threshold) continue; // fresh enough (or future-dated) - try out.append(allocator, .{ .account = e.account, .age_days = age, .hidden = hidden }); + try out.append(allocator, .{ .account = e.account, .age_days = age, .note = note }); } else { - try out.append(allocator, .{ .account = e.account, .age_days = null, .hidden = hidden }); + try out.append(allocator, .{ .account = e.account, .age_days = null, .note = note }); } } return out; } -/// Sort stale harvested figures worst-first: undated entries (which -/// never render), then oldest, then account name as a stable tiebreak. +/// Collect accounts whose `tax_mix_*` carve-outs are older than +/// `tax_mix_stale_days`, or that declare a mix with no `tax_mix_date`, +/// for the "Stale tax-mix figures" hygiene section. +/// +/// Same opt-in shape as `collectStaleHarvested`: only accounts that +/// declare a mix are considered, and future-dated entries fall out as +/// fresh so `doctor` owns reporting that typo. +/// +/// Accounts whose mix was *rejected* still count. The user clearly +/// meant to declare one, and `doctor` explains why it didn't take - +/// silently dropping the row would hide the account from both reports. +/// +/// Caller owns the returned list; `account` fields borrow from +/// `account_map`. +fn collectStaleTaxMix( + allocator: std.mem.Allocator, + account_map: analysis.AccountMap, + as_of: Date, +) !std.ArrayList(StaleDeclared) { + var out = std.ArrayList(StaleDeclared).empty; + errdefer out.deinit(allocator); + + const threshold: i32 = @intCast(tax_mix_stale_days); + for (account_map.entries) |e| { + if (!e.hasTaxMix()) continue; + + // No `note` counterpart to harvested's "no longer displayed": + // a stale tax mix keeps applying, by design. See + // `AccountTaxEntry.tax_mix_date`. + if (e.tax_mix_date) |on| { + const age = as_of.days - on.days; + if (age <= threshold) continue; // fresh enough (or future-dated) + try out.append(allocator, .{ .account = e.account, .age_days = age }); + } else { + try out.append(allocator, .{ .account = e.account, .age_days = null }); + } + } + return out; +} + +/// Sort stale hand-declared figures worst-first: undated entries (which +/// can't be aged at all), then oldest, then account name as a stable +/// tiebreak. /// /// Deliberately unlike `staleLessThan`, which sorts alphabetically: /// that section emits many rows per account and needs them grouped, -/// while this one emits exactly one row per account, so severity order +/// while these emit exactly one row per account, so severity order /// is strictly more useful. -fn harvestLessThan(_: void, a: StaleHarvest, b: StaleHarvest) bool { +fn staleDeclaredLessThan(_: void, a: StaleDeclared, b: StaleDeclared) bool { if (a.age_days == null and b.age_days != null) return true; if (a.age_days != null and b.age_days == null) return false; if (a.age_days) |ad| { @@ -528,6 +592,50 @@ fn countHarvestedAccounts(account_map: analysis.AccountMap) usize { return n; } +/// How many accounts declare `tax_mix_*` carve-outs at all. Same +/// use-the-feature-or-stay-quiet gate as `countHarvestedAccounts`. +fn countTaxMixAccounts(account_map: analysis.AccountMap) usize { + var n: usize = 0; + for (account_map.entries) |e| { + if (e.hasTaxMix()) n += 1; + } + return n; +} + +/// Render one "stale hand-declared figure" hygiene section. +/// +/// The two callers have identical shape - one row per account, worst +/// first - so the layout, the age coloring, and the reassuring `(none)` +/// live here. `title` is the whole section header; `undated_msg` is what +/// a row with no date says instead of an age. +/// +/// `rows` is expected pre-sorted by `staleDeclaredLessThan`. +fn printStaleDeclaredSection( + out: *std.Io.Writer, + color: bool, + rows: []const StaleDeclared, + threshold_days: u32, + title: []const u8, + undated_msg: []const u8, +) !void { + try out.print("\n", .{}); + try cli.printFg(out, color, cli.CLR_MUTED, " {s}\n", .{title}); + + if (rows.len == 0) { + try cli.printFg(out, color, cli.CLR_POSITIVE, " (none)\n", .{}); + return; + } + for (rows) |e| { + try out.print(" {s:<32} ", .{e.account}); + if (e.age_days) |ad| { + const clr = stalenessColor(ad, threshold_days); + try cli.printFg(out, color, clr, "last updated {d} days ago{s}\n", .{ @as(u32, @intCast(ad)), e.note }); + } else { + try cli.printFg(out, color, cli.CLR_NEGATIVE, "{s}\n", .{undated_msg}); + } + } +} + /// A lot whose manual `price` moved between HEAD and the working tree /// while its `price_date` stayed identical - the "bumped the price, /// forgot the date" mistake. String fields borrow from the working- @@ -966,25 +1074,58 @@ pub fn runHygieneCheck( if (declared > 0) { var stale_harvest = try collectStaleHarvested(allocator, account_map, as_of); defer stale_harvest.deinit(allocator); - std.mem.sort(StaleHarvest, stale_harvest.items, {}, harvestLessThan); + std.mem.sort(StaleDeclared, stale_harvest.items, {}, staleDeclaredLessThan); - try out.print("\n", .{}); - try cli.printFg(out, color, cli.CLR_MUTED, " Stale harvested figures (>{d} days - refresh 'harvested' in accounts.srf)\n", .{harvested_stale_days}); + var title_buf: [128]u8 = undefined; + const title = try std.fmt.bufPrint( + &title_buf, + "Stale harvested figures (>{d} days - refresh 'harvested' in accounts.srf)", + .{harvested_stale_days}, + ); + try printStaleDeclaredSection( + out, + color, + stale_harvest.items, + harvested_stale_days, + title, + "no harvested_date set", + ); + } + } - if (stale_harvest.items.len == 0) { - try cli.printFg(out, color, cli.CLR_POSITIVE, " (none)\n", .{}); - } else { - for (stale_harvest.items) |e| { - try out.print(" {s:<32} ", .{e.account}); - if (e.age_days) |ad| { - const clr = stalenessColor(ad, harvested_stale_days); - const suffix: []const u8 = if (e.hidden) " - no longer displayed" else ""; - try cli.printFg(out, color, clr, "last updated {d} days ago{s}\n", .{ @as(u32, @intCast(ad)), suffix }); - } else { - try cli.printFg(out, color, cli.CLR_NEGATIVE, "no harvested_date set\n", .{}); - } - } - } + // ── Section 3b: Stale tax-mix figures ── + // + // Same shape as Section 3, for the `tax_mix_*` carve-outs (see + // `analysis.TaxMix`). Kept separate from harvested for the same + // reason Section 3 is kept separate from Section 2: two different + // "days ago" meanings in one column mislead. + // + // Worth nagging about precisely because a stale mix never stops + // applying - it quietly feeds the By Tax Type breakdown forever. The + // alternative (retiring it like the harvested annotation) would + // silently move the user's pre-tax vs post-tax picture, which is + // worse than showing a slightly stale split. + { + const declared = countTaxMixAccounts(account_map); + if (declared > 0) { + var stale_mix = try collectStaleTaxMix(allocator, account_map, as_of); + defer stale_mix.deinit(allocator); + std.mem.sort(StaleDeclared, stale_mix.items, {}, staleDeclaredLessThan); + + var title_buf: [128]u8 = undefined; + const title = try std.fmt.bufPrint( + &title_buf, + "Stale tax-mix figures (>{d} days - refresh 'tax_mix_*' in accounts.srf)", + .{tax_mix_stale_days}, + ); + try printStaleDeclaredSection( + out, + color, + stale_mix.items, + tax_mix_stale_days, + title, + "no tax_mix_date set (mix still applies)", + ); } } @@ -1579,7 +1720,7 @@ test "collectStaleHarvested: exactly at the threshold is still fresh" { defer stale2.deinit(allocator); try std.testing.expectEqual(@as(usize, 1), stale2.items.len); try std.testing.expectEqual(@as(?i32, @intCast(harvested_stale_days + 1)), stale2.items[0].age_days); - try std.testing.expect(!stale2.items[0].hidden); + try std.testing.expectEqualStrings("", stale2.items[0].note); } test "collectStaleHarvested: undated figure is flagged with a null age" { @@ -1595,8 +1736,8 @@ test "collectStaleHarvested: undated figure is flagged with a null age" { try std.testing.expectEqual(@as(usize, 1), stale.items.len); try std.testing.expectEqualStrings("Sample Tax Loss", stale.items[0].account); try std.testing.expectEqual(@as(?i32, null), stale.items[0].age_days); - // An undated figure never renders, so it's hidden too. - try std.testing.expect(stale.items[0].hidden); + // An undated figure never renders, so it earns the retired note too. + try std.testing.expect(stale.items[0].note.len > 0); } test "collectStaleHarvested: future-dated figure is left to doctor" { @@ -1615,9 +1756,9 @@ test "collectStaleHarvested: future-dated figure is left to doctor" { try std.testing.expectEqual(@as(usize, 0), stale.items.len); } -test "collectStaleHarvested: hidden agrees with fmtHarvestAnnotation across the 12-month edge" { +test "collectStaleHarvested: note agrees with fmtHarvestAnnotation across the 12-month edge" { // The load-bearing invariant: the "no longer displayed" message must - // be true. `hidden` is derived from the formatter rather than from a + // be true. The note is derived from the formatter rather than from a // day count precisely so the two can't disagree at a leap-year // boundary. Walk a window that straddles the cutoff and assert // agreement on every day. @@ -1639,11 +1780,11 @@ test "collectStaleHarvested: hidden agrees with fmtHarvestAnnotation across the // SAFETY: immediately overwritten by fmtHarvestAnnotation. var buf: [fmt.harvest_annotation_max_len]u8 = undefined; const rendered = fmt.fmtHarvestAnnotation(&buf, 45_300, on, as_of); - try std.testing.expectEqual(rendered.len == 0, stale.items[0].hidden); + try std.testing.expectEqual(rendered.len == 0, stale.items[0].note.len > 0); } } -test "collectStaleHarvested: hidden is set once the annotation retires" { +test "collectStaleHarvested: note is set once the annotation retires" { const allocator = std.testing.allocator; var am = try testAccountMap( \\#!srfv1 @@ -1656,24 +1797,24 @@ test "collectStaleHarvested: hidden is set once the annotation retires" { defer stale.deinit(allocator); try std.testing.expectEqual(@as(usize, 1), stale.items.len); try std.testing.expectEqual(@as(?i32, 400), stale.items[0].age_days); - try std.testing.expect(stale.items[0].hidden); + try std.testing.expectEqualStrings(" - no longer displayed", stale.items[0].note); } -test "harvestLessThan: undated first, then oldest, then account name" { - const undated_b: StaleHarvest = .{ .account = "B", .age_days = null, .hidden = true }; - const undated_a: StaleHarvest = .{ .account = "A", .age_days = null, .hidden = true }; - const old: StaleHarvest = .{ .account = "C", .age_days = 400, .hidden = true }; - const newer: StaleHarvest = .{ .account = "D", .age_days = 100, .hidden = false }; +test "staleDeclaredLessThan: undated first, then oldest, then account name" { + const undated_b: StaleDeclared = .{ .account = "B", .age_days = null }; + const undated_a: StaleDeclared = .{ .account = "A", .age_days = null }; + const old: StaleDeclared = .{ .account = "C", .age_days = 400 }; + const newer: StaleDeclared = .{ .account = "D", .age_days = 100 }; // Undated outranks any dated entry, however old. - try std.testing.expect(harvestLessThan({}, undated_b, old)); - try std.testing.expect(!harvestLessThan({}, old, undated_b)); + try std.testing.expect(staleDeclaredLessThan({}, undated_b, old)); + try std.testing.expect(!staleDeclaredLessThan({}, old, undated_b)); // Among dated, older first. - try std.testing.expect(harvestLessThan({}, old, newer)); - try std.testing.expect(!harvestLessThan({}, newer, old)); + try std.testing.expect(staleDeclaredLessThan({}, old, newer)); + try std.testing.expect(!staleDeclaredLessThan({}, newer, old)); // Ties break on account name, so output is stable. - try std.testing.expect(harvestLessThan({}, undated_a, undated_b)); - try std.testing.expect(!harvestLessThan({}, undated_b, undated_a)); + try std.testing.expect(staleDeclaredLessThan({}, undated_a, undated_b)); + try std.testing.expect(!staleDeclaredLessThan({}, undated_b, undated_a)); } test "collectStaleHarvested + sort: mixed accounts come out worst-first" { @@ -1690,7 +1831,7 @@ test "collectStaleHarvested + sort: mixed accounts come out worst-first" { var stale = try collectStaleHarvested(allocator, am, Date.fromYmd(2026, 7, 25)); defer stale.deinit(allocator); - std.mem.sort(StaleHarvest, stale.items, {}, harvestLessThan); + std.mem.sort(StaleDeclared, stale.items, {}, staleDeclaredLessThan); try std.testing.expectEqual(@as(usize, 4), countHarvestedAccounts(am)); try std.testing.expectEqual(@as(usize, 3), stale.items.len); // Fresh and None excluded @@ -1698,8 +1839,200 @@ test "collectStaleHarvested + sort: mixed accounts come out worst-first" { try std.testing.expectEqualStrings("Sample Ancient", stale.items[1].account); try std.testing.expectEqualStrings("Sample Mild", stale.items[2].account); // Only the ancient one has retired from display. - try std.testing.expect(stale.items[1].hidden); - try std.testing.expect(!stale.items[2].hidden); + try std.testing.expect(stale.items[1].note.len > 0); + try std.testing.expectEqualStrings("", stale.items[2].note); +} + +// ── collectStaleTaxMix ─────────────────────────────────────── + +test "collectStaleTaxMix: no account declares a mix -> empty" { + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + + var stale = try collectStaleTaxMix(allocator, am, Date.fromYmd(2026, 7, 25)); + defer stale.deinit(allocator); + try std.testing.expectEqual(@as(usize, 0), stale.items.len); + try std.testing.expectEqual(@as(usize, 0), countTaxMixAccounts(am)); +} + +test "collectStaleTaxMix: a mix within the window is fresh" { + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-07-01 + ); + defer am.deinit(); + + var stale = try collectStaleTaxMix(allocator, am, Date.fromYmd(2026, 7, 25)); + defer stale.deinit(allocator); + try std.testing.expectEqual(@as(usize, 0), stale.items.len); + try std.testing.expectEqual(@as(usize, 1), countTaxMixAccounts(am)); +} + +test "collectStaleTaxMix: exactly at the threshold is still fresh" { + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-01-01 + ); + defer am.deinit(); + + const on = Date.fromYmd(2026, 1, 1); + const at_threshold = on.addDays(@intCast(tax_mix_stale_days)); + var stale = try collectStaleTaxMix(allocator, am, at_threshold); + defer stale.deinit(allocator); + try std.testing.expectEqual(@as(usize, 0), stale.items.len); + + // One day later it trips. + var stale2 = try collectStaleTaxMix(allocator, am, at_threshold.addDays(1)); + defer stale2.deinit(allocator); + try std.testing.expectEqual(@as(usize, 1), stale2.items.len); + try std.testing.expectEqual(@as(?i32, @intCast(tax_mix_stale_days + 1)), stale2.items[0].age_days); + // No "no longer displayed" counterpart: a stale mix keeps applying. + try std.testing.expectEqualStrings("", stale2.items[0].note); +} + +test "collectStaleTaxMix: an undated mix is flagged but never retires" { + // The load-bearing difference from `harvested`: an undated or ancient + // tax mix still feeds the By Tax Type breakdown. It gets a null age so + // it sorts first, but never a note claiming it stopped applying. + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4 + ); + defer am.deinit(); + + var stale = try collectStaleTaxMix(allocator, am, Date.fromYmd(2026, 7, 25)); + defer stale.deinit(allocator); + try std.testing.expectEqual(@as(usize, 1), stale.items.len); + try std.testing.expectEqualStrings("Sample 401k", stale.items[0].account); + try std.testing.expectEqual(@as(?i32, null), stale.items[0].age_days); + try std.testing.expectEqualStrings("", stale.items[0].note); + + // Years later, still applying and still noteless. + var ancient = try collectStaleTaxMix(allocator, am, Date.fromYmd(2030, 7, 25)); + defer ancient.deinit(allocator); + try std.testing.expectEqual(@as(usize, 1), ancient.items.len); + try std.testing.expectEqualStrings("", ancient.items[0].note); + try std.testing.expectApproxEqAbs( + @as(f64, 0.224), + am.entries[0].taxMix().weightOf(.roth), + 1e-12, + ); +} + +test "collectStaleTaxMix: future-dated mix is left to doctor" { + // Mirrors `collectStaleHarvested`: a future date is a config typo that + // `zfin doctor` already reports, so ageing it here would double-report. + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2027-01-01 + ); + defer am.deinit(); + + var stale = try collectStaleTaxMix(allocator, am, Date.fromYmd(2026, 7, 25)); + defer stale.deinit(allocator); + try std.testing.expectEqual(@as(usize, 0), stale.items.len); +} + +test "collectStaleTaxMix: a rejected mix is still collected" { + // The user meant to declare a mix, so the account belongs in the + // report even though the declaration didn't take. Dropping it would + // hide the account from the staleness nag AND leave doctor as the + // only mention. + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:140 + ); + defer am.deinit(); + + try std.testing.expectEqual(@as(usize, 1), countTaxMixAccounts(am)); + var stale = try collectStaleTaxMix(allocator, am, Date.fromYmd(2026, 7, 25)); + defer stale.deinit(allocator); + try std.testing.expectEqual(@as(usize, 1), stale.items.len); + try std.testing.expectEqual(@as(?i32, null), stale.items[0].age_days); +} + +test "collectStaleTaxMix + sort: mixed accounts come out worst-first" { + const allocator = std.testing.allocator; + var am = try testAccountMap( + \\#!srfv1 + \\account::Sample Fresh,tax_type::traditional,tax_mix_roth:num:10,tax_mix_date::2026-07-01 + \\account::Sample Mild,tax_type::traditional,tax_mix_roth:num:20,tax_mix_date::2026-01-01 + \\account::Sample Ancient,tax_type::traditional,tax_mix_roth:num:30,tax_mix_date::2024-01-01 + \\account::Sample Undated,tax_type::traditional,tax_mix_roth:num:40 + \\account::Sample None,tax_type::traditional + ); + defer am.deinit(); + + var stale = try collectStaleTaxMix(allocator, am, Date.fromYmd(2026, 7, 25)); + defer stale.deinit(allocator); + std.mem.sort(StaleDeclared, stale.items, {}, staleDeclaredLessThan); + + try std.testing.expectEqual(@as(usize, 4), countTaxMixAccounts(am)); + try std.testing.expectEqual(@as(usize, 3), stale.items.len); // Fresh and None excluded + try std.testing.expectEqualStrings("Sample Undated", stale.items[0].account); + try std.testing.expectEqualStrings("Sample Ancient", stale.items[1].account); + try std.testing.expectEqualStrings("Sample Mild", stale.items[2].account); +} + +// ── printStaleDeclaredSection ──────────────────────────────── + +test "printStaleDeclaredSection: no rows emits a reassuring (none)" { + // "You use this feature and everything is current" must be visibly + // different from the section being absent entirely. + var buf: [1024]u8 = undefined; + var w: std.Io.Writer = .fixed(&buf); + + try printStaleDeclaredSection(&w, false, &.{}, 90, "Stale widgets (>90 days)", "no widget_date set"); + + const out = w.buffered(); + try std.testing.expect(std.mem.indexOf(u8, out, "Stale widgets (>90 days)") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "(none)") != null); +} + +test "printStaleDeclaredSection: dated rows print an age, undated print the fallback" { + var buf: [2048]u8 = undefined; + var w: std.Io.Writer = .fixed(&buf); + + const rows = [_]StaleDeclared{ + .{ .account = "Sample Undated", .age_days = null }, + .{ .account = "Sample Ancient", .age_days = 400, .note = " - no longer displayed" }, + .{ .account = "Sample Mild", .age_days = 120 }, + }; + try printStaleDeclaredSection(&w, false, &rows, 90, "Stale harvested figures", "no harvested_date set"); + + const out = w.buffered(); + try std.testing.expect(std.mem.indexOf(u8, out, "Stale harvested figures") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "(none)") == null); + // Undated rows say why instead of showing a nonsense age. + try std.testing.expect(std.mem.indexOf(u8, out, "Sample Undated") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "no harvested_date set") != null); + // Dated rows show the age, and carry the note when one is set. + try std.testing.expect(std.mem.indexOf(u8, out, "last updated 400 days ago - no longer displayed") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "last updated 120 days ago\n") != null); +} + +test "printStaleDeclaredSection: an empty note appends nothing" { + // Regression guard for the tax-mix caller, which never sets a note: + // the age must not pick up stray trailing text. + var buf: [1024]u8 = undefined; + var w: std.Io.Writer = .fixed(&buf); + + const rows = [_]StaleDeclared{.{ .account = "Sample 401k", .age_days = 577 }}; + try printStaleDeclaredSection(&w, false, &rows, tax_mix_stale_days, "Stale tax-mix figures", "no tax_mix_date set (mix still applies)"); + + const out = w.buffered(); + try std.testing.expect(std.mem.indexOf(u8, out, "last updated 577 days ago\n") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "no longer displayed") == null); } // ── findPriceDateMismatches ────────────────────────────────── diff --git a/src/commands/doctor.zig b/src/commands/doctor.zig index ffeb2cc..e1cdde8 100644 --- a/src/commands/doctor.zig +++ b/src/commands/doctor.zig @@ -257,6 +257,99 @@ fn harvestedFieldChecks(arena: std.mem.Allocator, am: analysis.AccountMap, as_of }; } +/// Validate the `tax_mix_*` carve-outs in accounts.srf. +/// +/// Two distinct failure modes, reported in priority order: +/// +/// - **A rejected mix.** `AccountTaxEntry.taxMixChecked` refuses a +/// declaration that is non-positive, non-finite, names the account's +/// own `tax_type`, or over-allocates past 100 - and silently falls +/// back to the bare `tax_type`. That fallback is the whole reason +/// this check exists: without it the only signal is a log line the +/// user probably never sees, and their By Tax Type breakdown is +/// quietly not what they asked for. WARN. +/// - **A missing or future `tax_mix_date`.** Unlike `harvested`, a +/// dateless mix still applies, so this is advisory rather than +/// "your data is invisible". Still worth a WARN: an undated split +/// can't be aged, so `zfin audit` can never tell you it went stale. +/// +/// A merely *old* `tax_mix_date` is not flagged here - that's `audit`'s +/// job (`tax_mix_stale_days`), and duplicating it would double-report +/// the same routine maintenance. +fn taxMixFieldChecks(arena: std.mem.Allocator, am: analysis.AccountMap, as_of: Date) !Check { + const label = "accounts.srf tax mix"; + var rejected: std.ArrayList([]const u8) = .empty; + var undated: std.ArrayList([]const u8) = .empty; + var future: std.ArrayList([]const u8) = .empty; + var configured: usize = 0; + // Borrowed from `TaxMixProblem.label`; names the first cause found + // so the message says *why*, not just "invalid". + var first_cause: []const u8 = ""; + + for (am.entries) |e| { + if (!e.hasTaxMix()) continue; + configured += 1; + if (e.taxMixChecked().problem) |p| { + if (first_cause.len == 0) first_cause = p.label(); + try rejected.append(arena, e.account); + // A rejected mix's date is moot; don't pile on. + continue; + } + const on = e.tax_mix_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 = label, .detail = "no accounts declare a tax mix" }; + } + if (rejected.items.len > 0) { + return .{ + .status = .warn, + .label = label, + .detail = try std.fmt.allocPrint(arena, "{s} (falling back to tax_type): {s}", .{ + first_cause, + try joinCapped(arena, rejected.items, 6), + }), + }; + } + if (undated.items.len > 0 and future.items.len > 0) { + return .{ + .status = .warn, + .label = label, + .detail = try std.fmt.allocPrint(arena, "missing tax_mix_date: {s}; tax_mix_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 = label, + .detail = try std.fmt.allocPrint(arena, "tax mix declared but tax_mix_date missing (audit cannot age it): {s}", .{ + try joinCapped(arena, undated.items, 6), + }), + }; + } + if (future.items.len > 0) { + return .{ + .status = .warn, + .label = label, + .detail = try std.fmt.allocPrint(arena, "tax_mix_date is in the future: {s}", .{ + try joinCapped(arena, future.items, 6), + }), + }; + } + return .{ + .status = .ok, + .label = label, + .detail = try std.fmt.allocPrint(arena, "{d} account(s) with a valid, dated tax mix", .{configured}), + }; +} + /// 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 @@ -473,9 +566,11 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { 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)); + try checks.append(arena, try taxMixFieldChecks(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)" }); + try checks.append(arena, .{ .status = .info, .label = "accounts.srf tax mix", .detail = "skipped (accounts.srf not loaded)" }); } // Metadata (classification) coverage for classifiable holdings. @@ -924,6 +1019,137 @@ test "harvestedFieldChecks: both problems at once are reported together" { try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Trust") != null); } +test "taxMixFieldChecks: no account declares a mix -> info" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + const c = try taxMixFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.info, c.status); + try testing.expectEqualStrings("accounts.srf tax mix", c.label); +} + +test "taxMixFieldChecks: a valid dated mix is ok and counted" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2026-07-01 + \\account::Sample Brokerage,tax_type::taxable + ); + defer am.deinit(); + const c = try taxMixFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.ok, c.status); + try testing.expect(std.mem.indexOf(u8, c.detail, "1 account(s)") != null); +} + +test "taxMixFieldChecks: a merely old tax_mix_date is audit's job, not doctor's" { + // Staleness belongs to `zfin audit` (tax_mix_stale_days). Reporting it + // here too would double-nag on routine maintenance. + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2020-01-01 + ); + defer am.deinit(); + const c = try taxMixFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.ok, c.status); +} + +test "taxMixFieldChecks: a mix 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 401k,tax_type::traditional,tax_mix_roth:num:22.4 + ); + defer am.deinit(); + const c = try taxMixFieldChecks(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 401k") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "tax_mix_date") != null); +} + +test "taxMixFieldChecks: a future tax_mix_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 401k,tax_type::traditional,tax_mix_roth:num:22.4,tax_mix_date::2027-01-01 + ); + defer am.deinit(); + const c = try taxMixFieldChecks(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 401k") != null); +} + +test "taxMixFieldChecks: undated and future 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 401k,tax_type::traditional,tax_mix_roth:num:22.4 + \\account::Sample Other 401k,tax_type::traditional,tax_mix_roth:num:10,tax_mix_date::2027-01-01 + ); + defer am.deinit(); + const c = try taxMixFieldChecks(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 401k") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Other 401k") != null); +} + +test "taxMixFieldChecks: every rejection reason warns and says why" { + // A rejected mix silently falls back to the bare tax_type, so this + // check is the only place the user learns their split isn't applying. + // Each cause must name itself rather than collapsing to "invalid". + const cases = [_]struct { srf: []const u8, needle: []const u8 }{ + .{ .srf = "account::Sample 401k,tax_type::traditional,tax_mix_roth:num:0", .needle = "> 0" }, + .{ .srf = "account::Sample 401k,tax_type::traditional,tax_mix_roth:num:-5", .needle = "> 0" }, + .{ .srf = "account::Sample 401k,tax_type::traditional,tax_mix_roth:num:nan", .needle = "finite" }, + .{ .srf = "account::Sample 401k,tax_type::traditional,tax_mix_traditional:num:40", .needle = "own tax_type" }, + .{ .srf = "account::Sample 401k,tax_type::traditional,tax_mix_roth:num:140", .needle = "less than 100" }, + }; + for (cases) |case| { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const data = try std.fmt.allocPrint(arena.allocator(), "#!srfv1\n{s}\n", .{case.srf}); + var am = try analysis.parseAccountsFile(arena.allocator(), data); + defer am.deinit(); + + const c = try taxMixFieldChecks(arena.allocator(), am, Date.fromYmd(2026, 7, 25)); + try testing.expectEqual(Status.warn, c.status); + try testing.expect(std.mem.indexOf(u8, c.detail, case.needle) != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample 401k") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "falling back to tax_type") != null); + } +} + +test "taxMixFieldChecks: a rejected mix outranks a missing date" { + // Both accounts have something to report; the rejection is the more + // actionable one because it means the split isn't applying at all. + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var am = try analysis.parseAccountsFile(arena.allocator(), + \\#!srfv1 + \\account::Sample Undated,tax_type::traditional,tax_mix_roth:num:22.4 + \\account::Sample Broken,tax_type::traditional,tax_mix_roth:num:140 + ); + defer am.deinit(); + const c = try taxMixFieldChecks(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 Broken") != null); + try testing.expect(std.mem.indexOf(u8, c.detail, "falling back to tax_type") != null); + // The undated account is not mentioned; it'll surface once the + // rejection is fixed. + try testing.expect(std.mem.indexOf(u8, c.detail, "Sample Undated") == 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/views/review.zig b/src/views/review.zig index 213b34a..a11383e 100644 --- a/src/views/review.zig +++ b/src/views/review.zig @@ -11,7 +11,8 @@ //! classifications + `analytics/analysis.zig`'s sector bucketing. //! - **Tax%** - fraction of the holding's market value held in //! taxable accounts, computed by walking the per-lot `account` field -//! against `accounts.srf` (`AccountMap.taxTypeFor`). +//! against `accounts.srf`. Accounts with `tax_mix_*` carve-outs +//! contribute a fraction of their shares rather than all or none. //! - **Weight** - share of the liquid portfolio. //! - **Trailing returns** at 1Y/3Y/5Y/10Y, month-end total-return //! methodology (Morningstar-aligned). Falls back to adj_close @@ -502,6 +503,8 @@ fn bucketForSymbol( /// Walk the open lots for `symbol` (the allocation's `priceSymbol()`) /// and compute the share of market value held in taxable accounts. /// Lots are matched by `priceSymbol()`, so ticker-aliased lots count. +/// A lot in an account with `tax_mix_*` carve-outs contributes its +/// taxable fraction of the shares, not all or none. /// Returns null when: /// - account_map is null (no classification metadata available) /// - the symbol has no open lots @@ -526,23 +529,29 @@ fn computeTaxPct( if (!lot.lotIsOpenAsOf(as_of)) continue; if (lot.security_type != .stock) continue; // options/cash/CDs handled elsewhere const acct = lot.account orelse continue; - const is_taxable = isTaxableAccount(am, acct); - const is_classified = accountIsKnown(am, acct); - if (!is_classified) continue; + if (!accountIsKnown(am, acct)) continue; classified_shares += lot.shares; - if (is_taxable) taxable_shares += lot.shares; + taxable_shares += lot.shares * taxableWeight(am, acct); } if (classified_shares <= 0) return null; return taxable_shares / classified_shares; } -fn isTaxableAccount(am: analysis.AccountMap, account: []const u8) bool { +/// What share of `account`'s value is `taxable` money, in [0, 1]. +/// +/// Usually 0 or 1. It is a fraction for an account with `tax_mix_*` +/// carve-outs in accounts.srf - e.g. a 401(k) whose after-tax non-Roth +/// sleeve is taxable-basis money lumped into one reported balance. +/// Returns 0 for accounts absent from the map, though callers should +/// screen those out with `accountIsKnown` first so an unclassified +/// account doesn't read as "definitely not taxable". +fn taxableWeight(am: analysis.AccountMap, account: []const u8) f64 { for (am.entries) |e| { if (std.mem.eql(u8, e.account, account)) { - return e.tax_type == .taxable; + return e.taxMix().weightOf(.taxable); } } - return false; + return 0; } fn accountIsKnown(am: analysis.AccountMap, account: []const u8) bool { @@ -917,6 +926,64 @@ test "computeTaxPct: mixed accounts produces partial tax%" { try testing.expectApproxEqAbs(@as(f64, 0.6), result.?, 0.001); } +test "computeTaxPct: a tax_mix carve-out contributes a fraction of its shares" { + // An account with `tax_mix_taxable` carve-outs (a 401(k) with an + // after-tax non-Roth sleeve) is partly taxable, so its shares count + // fractionally rather than all or none. + var lots = [_]zfin.Lot{ + .{ .symbol = "VTI", .shares = 100, .open_date = Date.fromYmd(2022, 1, 10), .open_price = 200, .account = "Sample Brokerage" }, + .{ .symbol = "VTI", .shares = 400, .open_date = Date.fromYmd(2022, 1, 10), .open_price = 200, .account = "Sample 401k" }, + }; + const portfolio: zfin.Portfolio = .{ .lots = lots[0..], .allocator = testing.allocator }; + var entries = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample Brokerage", .tax_type = .taxable }, + .{ .account = "Sample 401k", .tax_type = .traditional, .tax_mix_taxable = 25 }, + }; + const am: analysis.AccountMap = .{ .entries = entries[0..], .allocator = testing.allocator }; + const result = computeTaxPct("VTI", portfolio, am, Date.fromYmd(2026, 1, 1)); + // Brokerage: 100 taxable. 401k: 400 * 25% = 100 taxable. + // (100 + 100) / 500 = 0.4 + try testing.expectApproxEqAbs(@as(f64, 0.4), result.?, 0.001); +} + +test "computeTaxPct: a traditional/roth mix leaves Tax% unchanged" { + // Neither half is taxable, so the common mix must be inert here. + // Pins the claim that Tax% only moves for a taxable carve-out. + var lots = [_]zfin.Lot{ + .{ .symbol = "VTI", .shares = 60, .open_date = Date.fromYmd(2022, 1, 10), .open_price = 200, .account = "Sample Brokerage" }, + .{ .symbol = "VTI", .shares = 40, .open_date = Date.fromYmd(2022, 1, 10), .open_price = 200, .account = "Sample 401k" }, + }; + const portfolio: zfin.Portfolio = .{ .lots = lots[0..], .allocator = testing.allocator }; + var mixed = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample Brokerage", .tax_type = .taxable }, + .{ .account = "Sample 401k", .tax_type = .traditional, .tax_mix_roth = 22.4 }, + }; + var plain = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample Brokerage", .tax_type = .taxable }, + .{ .account = "Sample 401k", .tax_type = .traditional }, + }; + const as_of = Date.fromYmd(2026, 1, 1); + const a = computeTaxPct("VTI", portfolio, .{ .entries = mixed[0..], .allocator = testing.allocator }, as_of); + const b = computeTaxPct("VTI", portfolio, .{ .entries = plain[0..], .allocator = testing.allocator }, as_of); + try testing.expectEqual(b, a); + try testing.expectApproxEqAbs(@as(f64, 0.6), a.?, 0.001); +} + +test "computeTaxPct: an invalid tax mix falls back to the bare tax_type" { + // A rejected carve-out (here: it names the account's own tax_type) + // must read as fully taxable, exactly as if it were absent. + var lots = [_]zfin.Lot{ + .{ .symbol = "VTI", .shares = 100, .open_date = Date.fromYmd(2022, 1, 10), .open_price = 200, .account = "Sample Brokerage" }, + }; + const portfolio: zfin.Portfolio = .{ .lots = lots[0..], .allocator = testing.allocator }; + var entries = [_]analysis.AccountTaxEntry{ + .{ .account = "Sample Brokerage", .tax_type = .taxable, .tax_mix_taxable = 40 }, + }; + const am: analysis.AccountMap = .{ .entries = entries[0..], .allocator = testing.allocator }; + const result = computeTaxPct("VTI", portfolio, am, Date.fromYmd(2026, 1, 1)); + try testing.expectApproxEqAbs(@as(f64, 1.0), result.?, 0.001); +} + test "computeTaxPct: matches ticker-aliased lots via priceSymbol()" { // Regression: the caller passes the allocation's symbol, which is // priceSymbol(). A CUSIP lot aliased to a ticker must be matched