tighten manual price hygiene / compare to penny on cash lots in audit
This commit is contained in:
parent
1c4f85f8da
commit
2268f1dfd0
GPG key ID:
A7B62D657EF764F8
2 changed files with 557 additions and 136 deletions
124
TODO.md
124
TODO.md
|
|
@ -240,6 +240,36 @@ hygiene without inheriting the reconciliation surface.
|
|||
|
||||
Pure internal refactor; no user-visible change.
|
||||
|
||||
## Audit: reconcile accounts present in the portfolio but absent from the export — priority MEDIUM
|
||||
|
||||
`compareAccounts` (and `compareSchwabSummary`) iterate over the
|
||||
accounts found in the *brokerage export*, then look up the matching
|
||||
portfolio account. The two directions are asymmetric:
|
||||
|
||||
- **Export row → no portfolio account:** handled. The
|
||||
`portfolio_acct_name == null` branch surfaces it as
|
||||
"unmapped — add account_number to accounts.srf" and flags a
|
||||
discrepancy.
|
||||
- **Portfolio account → not in the export:** *silent gap.* An
|
||||
account that exists in `accounts.srf` / has lots in
|
||||
`portfolio.srf` but has no corresponding account in the CSV is
|
||||
never iterated, so the reconciler says nothing. If you forget to
|
||||
include an account in the download, or a brokerage drops it from
|
||||
the export, audit can't tell you "you hold account X that wasn't
|
||||
in this file — stale, or just not exported?"
|
||||
|
||||
Fix sketch: after the per-export-account loop, walk the
|
||||
`account_map` entries for the institution being reconciled and, for
|
||||
any whose portfolio account holds open lots as-of but never appeared
|
||||
in the export, emit a "portfolio account not found in export" notice.
|
||||
Gate it to the institution under audit (don't flag Schwab accounts
|
||||
when reconciling a Fidelity export). Decide whether a zero-balance /
|
||||
fully-closed account should be suppressed.
|
||||
|
||||
Found while debugging a BrokerageLink cash reconciliation — that
|
||||
account *was* in the export, so this gap wasn't the culprit, but the
|
||||
asymmetry is real and worth closing.
|
||||
|
||||
## Refactor: trim `src/format.zig` once Money / Date have absorbed their helpers — priority LOW
|
||||
|
||||
`src/format.zig` is still a ~1700-line grab-bag, but the money- and
|
||||
|
|
@ -300,100 +330,6 @@ change inputs to either of these loaders, change them in BOTH
|
|||
places." Adding a third copy of the imported-only loader code
|
||||
makes that worse.
|
||||
|
||||
## Audit: manual-check accounts mechanism — priority HIGH
|
||||
|
||||
Some accounts/positions can't be reconciled from broker CSVs and need a
|
||||
human-in-the-loop reminder at the audit step. Two recurring shapes:
|
||||
|
||||
- **No-CSV-export accounts** (e.g. some insurance / annuity products)
|
||||
where values only live in periodic statements. Git can't detect a
|
||||
"change" because nothing changes locally; the user has to log in
|
||||
to see the new value.
|
||||
- **Payroll-deduction-then-purchase accounts** (e.g. ESPP) where
|
||||
payroll-deducted cash doesn't appear in the broker positions CSV
|
||||
until the purchase date hits (typically every 6 months). Between
|
||||
purchases the cash is a real contribution that `zfin audit` can't
|
||||
see.
|
||||
|
||||
The existing `update_cadence::weekly|monthly|quarterly|none` field already
|
||||
sort-of covers this, but has two gaps:
|
||||
|
||||
1. It fires off the last *git-detected change*, not the last *human
|
||||
review*. For statement-only accounts, the value sometimes hasn't
|
||||
changed in months — so git never fires, cadence never trips.
|
||||
2. Payroll-deduction accounts need weekly-ish attention while
|
||||
accumulating cash between purchases, but the accrued balance is
|
||||
invisible to the CSV audit.
|
||||
|
||||
Drift symptom seen in practice: several accounts on
|
||||
`update_cadence::weekly` in `accounts.srf` weren't flagged as overdue
|
||||
despite no changes in two weeks, because the cadence reads
|
||||
git-detected change time rather than human-review time. The cadence
|
||||
values themselves may also be wrong for these accounts — revisit
|
||||
whether weekly is the right cadence vs. monthly/quarterly given how
|
||||
rarely they actually change.
|
||||
|
||||
### Options
|
||||
|
||||
A. **New `update_cadence::manual` variant** — always fires every audit
|
||||
run until silenced. Blunt but zero design work.
|
||||
|
||||
B. **`last_refreshed::YYYY-MM-DD` field on `accounts.srf`** — explicit
|
||||
human-review timestamp, decoupled from git-detected changes. Audit
|
||||
compares `today - last_refreshed` against the cadence. User bumps
|
||||
the field when they check the statement. Probably the most
|
||||
correct fit for statement-only accounts.
|
||||
|
||||
C. **Sticky TODO list** — a `todos.srf` or `todo::` field on accounts
|
||||
that audit always surfaces until cleared. General-purpose; also
|
||||
covers "remember to rebalance on 5/15".
|
||||
|
||||
### ESPP-style accrual follow-through
|
||||
|
||||
Payroll-deduction accounts are also a contribution-attribution blind
|
||||
spot. If a paystub deducts $X/week but the cash lot doesn't reach
|
||||
`portfolio.srf` until the purchase date, the attribution math is
|
||||
under-counting contributions and over-counting the purchase-week
|
||||
gain. Possible fixes are discussed in the "Contributions diff" TODO
|
||||
below — option C there (per-account `cash_is_contribution`) would
|
||||
make manually-entered ESPP-style cash additions count correctly.
|
||||
|
||||
## Audit: stale manual prices section is incorrect — priority HIGH
|
||||
|
||||
The `Stale manual prices` section in `zfin audit` (in
|
||||
`src/commands/audit.zig` around line 1333) isn't computing the
|
||||
right thing. The current logic walks `portfolio.lots`, filters to
|
||||
lots with both `price` and `price_date` set, and flags any whose
|
||||
`as_of.days - price_date.days > stale_days`. In practice this
|
||||
either over-flags (counting lots that aren't really
|
||||
manually-priced), under-flags (missing lots that ARE manually
|
||||
priced but lack `price_date`), or both — needs investigation
|
||||
against a real portfolio to determine which.
|
||||
|
||||
Things to check:
|
||||
|
||||
- Are we using the right field to identify "manually priced"? The
|
||||
`Lot.price` field is set for any non-API price (manual override,
|
||||
illiquid valuation, CD face, etc.); some of those shouldn't be
|
||||
in a "stale prices" check (e.g. CDs with a fixed face value
|
||||
aren't stale by age).
|
||||
- Should the staleness comparison use `Allocation.is_manual_price`
|
||||
(computed at the position level after the price-resolution
|
||||
cascade) instead of the per-lot field? That captures "the price
|
||||
this position is currently displaying came from a manual
|
||||
source," which is what the user actually cares about.
|
||||
- `price_date` falsely-null lots: if a lot has `price` set but no
|
||||
`price_date`, we silently skip it instead of flagging it. That's
|
||||
almost certainly wrong — a manually-priced lot with no recorded
|
||||
date is the *most* stale case, not the least.
|
||||
- Per-symbol vs per-lot: if the same symbol appears in multiple
|
||||
lots with the same manual price, we currently emit one line per
|
||||
lot. Probably wants to be one line per symbol with a count, or
|
||||
at least dedup by `(symbol, price, price_date)`.
|
||||
|
||||
Fix should land with regression tests against a fixture portfolio
|
||||
that exhibits each of the above shapes.
|
||||
|
||||
## Investigate: detailed 401(k) contributions data source
|
||||
|
||||
Found a more detailed contributions screen on at least one
|
||||
|
|
|
|||
|
|
@ -22,6 +22,20 @@ const parseSchwabCsv = schwab.parseCsv;
|
|||
const parseSchwabSummary = schwab.parseSummary;
|
||||
const fidelityOptionMatchesLot = fidelity.optionMatchesLot;
|
||||
|
||||
/// Reconciliation match tolerances.
|
||||
///
|
||||
/// Securities get $1 of slack to absorb NAV-rounding on large
|
||||
/// positions: a sub-cent per-share NAV difference between the broker
|
||||
/// and zfin's fetched price on a six-figure mutual-fund position
|
||||
/// easily exceeds a dollar, and that's not an actionable discrepancy.
|
||||
///
|
||||
/// Cash is different — it has no NAV and no share count, it's an exact
|
||||
/// dollar figure on both sides. It must match to the penny; the $1
|
||||
/// securities slack would otherwise silently hide real money-market
|
||||
/// dividend accrual between updates (the whole point of the audit).
|
||||
const value_tolerance: f64 = 1.0;
|
||||
const cash_tolerance: f64 = 0.01;
|
||||
|
||||
/// Account-level comparison result for Schwab summary audit.
|
||||
pub const SchwabAccountComparison = struct {
|
||||
account_name: []const u8,
|
||||
|
|
@ -92,8 +106,8 @@ pub fn compareSchwabSummary(
|
|||
const cash_delta = if (sa.cash) |sc| sc - pf_cash else null;
|
||||
const total_delta = if (sa.total_value) |st| st - pf_total else null;
|
||||
|
||||
const cash_ok = if (cash_delta) |d| @abs(d) < 1.0 else true;
|
||||
const total_ok = if (total_delta) |d| @abs(d) < 1.0 else true;
|
||||
const cash_ok = if (cash_delta) |d| @abs(d) < cash_tolerance else true;
|
||||
const total_ok = if (total_delta) |d| @abs(d) < value_tolerance else true;
|
||||
|
||||
try results.append(allocator, .{
|
||||
.account_name = portfolio_acct orelse "",
|
||||
|
|
@ -141,8 +155,8 @@ fn displaySchwabResults(results: []const SchwabAccountComparison, color: bool, o
|
|||
else
|
||||
"--";
|
||||
|
||||
const cash_ok = if (r.cash_delta) |d| @abs(d) < 1.0 else true;
|
||||
const total_ok = if (r.total_delta) |d| @abs(d) < 1.0 else true;
|
||||
const cash_ok = if (r.cash_delta) |d| @abs(d) < cash_tolerance else true;
|
||||
const total_ok = if (r.total_delta) |d| @abs(d) < value_tolerance else true;
|
||||
const is_unmapped = r.account_name.len == 0;
|
||||
const is_real_mismatch = !cash_ok or is_unmapped;
|
||||
|
||||
|
|
@ -484,7 +498,9 @@ pub fn compareAccounts(
|
|||
const value_delta = if (bp.current_value) |bv| bv - pf_value else null;
|
||||
|
||||
const shares_match = if (shares_delta) |d| @abs(d) < 0.01 else true;
|
||||
const value_match = if (value_delta) |d| @abs(d) < 1.0 else true;
|
||||
// Cash matches to the penny; securities get $1 of NAV-rounding slack.
|
||||
const tol: f64 = if (bp.is_cash) cash_tolerance else value_tolerance;
|
||||
const value_match = if (value_delta) |d| @abs(d) < tol else true;
|
||||
|
||||
// Option value deltas are expected (cost basis vs mark-to-market)
|
||||
// — track them separately rather than flagging as discrepancies
|
||||
|
|
@ -839,7 +855,7 @@ fn displayResults(results: []const AccountComparison, color: bool, out: *std.Io.
|
|||
|
||||
// Classify this row
|
||||
const shares_ok = if (cmp.shares_delta) |d| @abs(d) < 0.01 else !cmp.only_in_brokerage;
|
||||
const is_cash_mismatch = cmp.is_cash and (if (cmp.value_delta) |d| @abs(d) >= 1.0 else false);
|
||||
const is_cash_mismatch = cmp.is_cash and (if (cmp.value_delta) |d| @abs(d) >= cash_tolerance else false);
|
||||
const is_real_mismatch = !shares_ok or cmp.only_in_brokerage or cmp.only_in_portfolio or is_cash_mismatch;
|
||||
|
||||
if (is_real_mismatch) discrepancy_count += 1;
|
||||
|
|
@ -910,7 +926,7 @@ fn displayResults(results: []const AccountComparison, color: bool, out: *std.Io.
|
|||
|
||||
// Shares match — show value delta (stale price) if any, muted
|
||||
if (cmp.value_delta) |d| {
|
||||
if (@abs(d) >= 1.0) {
|
||||
if (@abs(d) >= value_tolerance) {
|
||||
const sign: []const u8 = if (d >= 0) "+" else "-";
|
||||
break :blk std.fmt.bufPrint(&status_buf, "Value {s}{f}", .{ sign, Money.from(@abs(d)) }) catch "";
|
||||
}
|
||||
|
|
@ -1231,6 +1247,185 @@ fn stalenessColor(age_days: i32, threshold: u32) [3]u8 {
|
|||
return cli.CLR_NEGATIVE;
|
||||
}
|
||||
|
||||
/// One stale (or undated) manual price found during the hygiene scan.
|
||||
/// String fields borrow from the scanned portfolio's lots and are
|
||||
/// valid for the lifetime of that portfolio.
|
||||
const StaleManualPrice = struct {
|
||||
account: []const u8,
|
||||
symbol: []const u8,
|
||||
note: ?[]const u8,
|
||||
price: f64,
|
||||
/// `null` when the lot carries a manual `price` but no
|
||||
/// `price_date` — the most-stale case (it can't even be aged),
|
||||
/// not the least.
|
||||
price_date: ?Date,
|
||||
/// Days since `price_date`; `null` when undated.
|
||||
age_days: ?i32,
|
||||
};
|
||||
|
||||
/// Collect manual-priced lots that are stale (older than `stale_days`)
|
||||
/// or undated, for the "Stale manual prices" hygiene section.
|
||||
///
|
||||
/// Staleness is purely a property of the `price` / `price_date` the
|
||||
/// user typed on the lot — it has nothing to do with the account's
|
||||
/// `update_cadence` (that's the reconciliation cadence, a separate
|
||||
/// concept handled by the "Accounts overdue" section). The single
|
||||
/// threshold is `stale_days` (the `--stale-days` flag, default 3).
|
||||
///
|
||||
/// Restricted to open `security_type == .stock` lots: CDs/cash/options
|
||||
/// carry `price` as a fixed face value that never goes "stale by age,"
|
||||
/// and closed lots aren't worth nagging about. Undated manual prices
|
||||
/// are always included regardless of `stale_days` — a manual price
|
||||
/// with no `price_date` can't be aged, which is the worst case, not a
|
||||
/// pass. Caller owns the returned list; string fields borrow from
|
||||
/// `portfolio`.
|
||||
fn collectStaleManualPrices(
|
||||
allocator: std.mem.Allocator,
|
||||
portfolio: zfin.Portfolio,
|
||||
as_of: Date,
|
||||
stale_days: u32,
|
||||
) !std.ArrayList(StaleManualPrice) {
|
||||
var out = std.ArrayList(StaleManualPrice).empty;
|
||||
errdefer out.deinit(allocator);
|
||||
|
||||
const threshold: i32 = @intCast(stale_days);
|
||||
for (portfolio.lots) |lot| {
|
||||
if (lot.security_type != .stock) continue;
|
||||
const price = lot.price orelse continue;
|
||||
if (!lot.isOpen(as_of)) continue;
|
||||
const account = lot.account orelse "(no account)";
|
||||
|
||||
if (lot.price_date) |pd| {
|
||||
const age = as_of.days - pd.days;
|
||||
if (age <= threshold) continue; // fresh enough
|
||||
try out.append(allocator, .{
|
||||
.account = account,
|
||||
.symbol = lot.symbol,
|
||||
.note = lot.note,
|
||||
.price = price,
|
||||
.price_date = pd,
|
||||
.age_days = age,
|
||||
});
|
||||
} else {
|
||||
try out.append(allocator, .{
|
||||
.account = account,
|
||||
.symbol = lot.symbol,
|
||||
.note = lot.note,
|
||||
.price = price,
|
||||
.price_date = null,
|
||||
.age_days = null,
|
||||
});
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/// Sort stale manual prices by account, then symbol — so the display
|
||||
/// can group lines under per-account headers.
|
||||
fn staleLessThan(_: void, a: StaleManualPrice, b: StaleManualPrice) bool {
|
||||
const acc = std.mem.order(u8, a.account, b.account);
|
||||
if (acc != .eq) return acc == .lt;
|
||||
return std.mem.order(u8, a.symbol, b.symbol) == .lt;
|
||||
}
|
||||
|
||||
/// 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-
|
||||
/// tree portfolio.
|
||||
const PriceDateMismatch = struct {
|
||||
account: []const u8,
|
||||
symbol: []const u8,
|
||||
old_price: f64,
|
||||
new_price: f64,
|
||||
price_date: Date,
|
||||
};
|
||||
|
||||
/// Build a lot identity that survives a manual-price edit: a lot keeps
|
||||
/// its symbol, account, open_date, and open_price when you only change
|
||||
/// `price`/`price_date`. Used to pair HEAD lots with working-tree lots.
|
||||
/// Caller owns the returned slice.
|
||||
fn lotIdentityKey(allocator: std.mem.Allocator, lot: portfolio_mod.Lot) ![]const u8 {
|
||||
return std.fmt.allocPrint(allocator, "{s}\x00{s}\x00{d}\x00{d:.6}", .{
|
||||
lot.symbol,
|
||||
lot.account orelse "",
|
||||
lot.open_date.days,
|
||||
lot.open_price,
|
||||
});
|
||||
}
|
||||
|
||||
/// Find lots whose manual `price` changed between `committed` (HEAD)
|
||||
/// and `working` (on-disk) while `price_date` stayed identical.
|
||||
///
|
||||
/// This is the working-tree-vs-HEAD detector for the recurring "I
|
||||
/// updated the price but forgot the date" mistake, run when `audit`
|
||||
/// fires before a commit. Only open `security_type == .stock` lots
|
||||
/// with a non-null `price_date` on both sides participate: the undated
|
||||
/// case is reported by `collectStaleManualPrices`, and legitimate
|
||||
/// back-dating (moving the date to a past close) is *not* flagged
|
||||
/// because the date field changed. Lots are paired by
|
||||
/// `lotIdentityKey`; newly-added or removed lots are ignored. Caller
|
||||
/// owns the returned list; string fields borrow from `working`.
|
||||
fn findPriceDateMismatches(
|
||||
allocator: std.mem.Allocator,
|
||||
committed: zfin.Portfolio,
|
||||
working: zfin.Portfolio,
|
||||
as_of: Date,
|
||||
) !std.ArrayList(PriceDateMismatch) {
|
||||
var out = std.ArrayList(PriceDateMismatch).empty;
|
||||
errdefer out.deinit(allocator);
|
||||
|
||||
// Index HEAD lots by stable identity → their (price, price_date).
|
||||
const HeadLot = struct { price: ?f64, price_date: ?Date };
|
||||
var head = std.StringHashMap(HeadLot).init(allocator);
|
||||
defer {
|
||||
var it = head.keyIterator();
|
||||
while (it.next()) |k| allocator.free(k.*);
|
||||
head.deinit();
|
||||
}
|
||||
for (committed.lots) |lot| {
|
||||
if (lot.security_type != .stock) continue;
|
||||
const key = try lotIdentityKey(allocator, lot);
|
||||
const gop = try head.getOrPut(key);
|
||||
if (gop.found_existing) {
|
||||
// Duplicate identity (rare) — ambiguous, don't guess.
|
||||
allocator.free(key);
|
||||
continue;
|
||||
}
|
||||
gop.value_ptr.* = .{ .price = lot.price, .price_date = lot.price_date };
|
||||
}
|
||||
|
||||
for (working.lots) |lot| {
|
||||
if (lot.security_type != .stock) continue;
|
||||
if (!lot.isOpen(as_of)) continue;
|
||||
const new_price = lot.price orelse continue;
|
||||
const new_date = lot.price_date orelse continue; // undated → stale-price section's job
|
||||
const key = try lotIdentityKey(allocator, lot);
|
||||
defer allocator.free(key);
|
||||
const prior = head.get(key) orelse continue; // newly-added lot
|
||||
const old_price = prior.price orelse continue; // price added, not moved
|
||||
const old_date = prior.price_date orelse continue; // was undated before
|
||||
// The mistake: price moved, date did not.
|
||||
if (old_date.days == new_date.days and @abs(old_price - new_price) >= 0.005) {
|
||||
try out.append(allocator, .{
|
||||
.account = lot.account orelse "(no account)",
|
||||
.symbol = lot.symbol,
|
||||
.old_price = old_price,
|
||||
.new_price = new_price,
|
||||
.price_date = new_date,
|
||||
});
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/// Sort price/date mismatches by account, then symbol — for grouped
|
||||
/// per-account display.
|
||||
fn mismatchLessThan(_: void, a: PriceDateMismatch, b: PriceDateMismatch) bool {
|
||||
const acc = std.mem.order(u8, a.account, b.account);
|
||||
if (acc != .eq) return acc == .lt;
|
||||
return std.mem.order(u8, a.symbol, b.symbol) == .lt;
|
||||
}
|
||||
|
||||
/// Render one unmatched large-lot warning. Formats the line the
|
||||
/// user needs to paste into `transaction_log.srf` if the lot was
|
||||
/// an internal movement rather than a real external contribution.
|
||||
|
|
@ -1331,45 +1526,47 @@ fn runHygieneCheck(
|
|||
try cli.printBold(out, color, " Portfolio hygiene\n", .{});
|
||||
|
||||
// ── Section 1: Stale manual prices ──
|
||||
|
||||
var stale_count: usize = 0;
|
||||
|
||||
// Collect and display stale manual prices
|
||||
//
|
||||
// Manual prices on stock/fund lots whose `price_date` is older than
|
||||
// `--stale-days` (default 3), plus manual prices with no
|
||||
// `price_date` at all (the most-stale case). Staleness is a
|
||||
// property of the price the user typed on the lot — not of the
|
||||
// account; grouping by account here is display-only organization.
|
||||
// CDs/cash/options are excluded — their `price` is a fixed face
|
||||
// value, not an age-stale quote — as are closed lots.
|
||||
{
|
||||
var header_shown = false;
|
||||
for (portfolio.lots) |lot| {
|
||||
if (lot.price == null) continue;
|
||||
const pd = lot.price_date orelse continue;
|
||||
const age_days = as_of.days - pd.days;
|
||||
const threshold: i32 = @intCast(stale_days);
|
||||
if (age_days <= threshold) continue;
|
||||
var stale = try collectStaleManualPrices(allocator, portfolio, as_of, stale_days);
|
||||
defer stale.deinit(allocator);
|
||||
std.mem.sort(StaleManualPrice, stale.items, {}, staleLessThan);
|
||||
|
||||
if (!header_shown) {
|
||||
try out.print("\n", .{});
|
||||
try cli.printFg(out, color, cli.CLR_MUTED, " Stale manual prices (>{d} days — --stale-days to configure)\n", .{stale_days});
|
||||
header_shown = true;
|
||||
}
|
||||
try out.print("\n", .{});
|
||||
try cli.printFg(out, color, cli.CLR_MUTED, " Stale manual prices (>{d} days — --stale-days to configure)\n", .{stale_days});
|
||||
|
||||
stale_count += 1;
|
||||
var date_buf: [10]u8 = undefined;
|
||||
const date_str = std.fmt.bufPrint(&date_buf, "{f}", .{pd}) catch "????-??-??";
|
||||
const note_display = lot.note orelse "";
|
||||
var price_buf: [24]u8 = undefined;
|
||||
const price_str = std.fmt.bufPrint(&price_buf, "{f}", .{Money.from(lot.price.?)}) catch "$?";
|
||||
|
||||
try out.print(" {s:<16} {s:<16} {s:>10} {s} ", .{
|
||||
lot.symbol,
|
||||
note_display,
|
||||
price_str,
|
||||
date_str,
|
||||
});
|
||||
const clr = stalenessColor(age_days, stale_days);
|
||||
try cli.printFg(out, color, clr, "({d} days)\n", .{@as(u32, @intCast(age_days))});
|
||||
}
|
||||
if (!header_shown) {
|
||||
try out.print("\n", .{});
|
||||
try cli.printFg(out, color, cli.CLR_MUTED, " Stale manual prices (>{d} days)\n", .{stale_days});
|
||||
if (stale.items.len == 0) {
|
||||
try cli.printFg(out, color, cli.CLR_POSITIVE, " (none)\n", .{});
|
||||
} else {
|
||||
var current_account: ?[]const u8 = null;
|
||||
for (stale.items) |e| {
|
||||
if (current_account == null or !std.mem.eql(u8, current_account.?, e.account)) {
|
||||
current_account = e.account;
|
||||
try cli.printFg(out, color, cli.CLR_HEADER, " {s}\n", .{e.account});
|
||||
}
|
||||
|
||||
var price_buf: [24]u8 = undefined;
|
||||
const price_str = std.fmt.bufPrint(&price_buf, "{f}", .{Money.from(e.price)}) catch "$?";
|
||||
const note_display = e.note orelse "";
|
||||
|
||||
if (e.price_date) |pd| {
|
||||
var date_buf: [10]u8 = undefined;
|
||||
const date_str = std.fmt.bufPrint(&date_buf, "{f}", .{pd}) catch "????-??-??";
|
||||
try out.print(" {s:<14} {s:<16} {s:>12} {s} ", .{ e.symbol, note_display, price_str, date_str });
|
||||
const clr = stalenessColor(e.age_days.?, stale_days);
|
||||
try cli.printFg(out, color, clr, "({d} days)\n", .{@as(u32, @intCast(e.age_days.?))});
|
||||
} else {
|
||||
try out.print(" {s:<14} {s:<16} {s:>12} ", .{ e.symbol, note_display, price_str });
|
||||
try cli.printFg(out, color, cli.CLR_NEGATIVE, "(no price_date set)\n", .{});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1397,6 +1594,41 @@ fn runHygieneCheck(
|
|||
}
|
||||
}
|
||||
|
||||
// ── Section 1b: manual price changed without bumping price_date ──
|
||||
//
|
||||
// Catches the recurring "I updated the price but forgot the
|
||||
// date" mistake at the moment it matters — when `audit` runs
|
||||
// against the working tree before a commit. Diffs the on-disk
|
||||
// portfolio against HEAD; a lot whose `price` moved while its
|
||||
// `price_date` stayed put is flagged. Silent when there's no
|
||||
// committed version to compare against (not a repo / new file)
|
||||
// and when no such mismatch exists.
|
||||
if (committed_portfolio) |cp| {
|
||||
var mismatches = try findPriceDateMismatches(allocator, cp, portfolio, as_of);
|
||||
defer mismatches.deinit(allocator);
|
||||
std.mem.sort(PriceDateMismatch, mismatches.items, {}, mismatchLessThan);
|
||||
|
||||
if (mismatches.items.len > 0) {
|
||||
try out.print("\n", .{});
|
||||
try cli.printFg(out, color, cli.CLR_MUTED, " Manual price changed without updating price_date (working tree vs HEAD)\n", .{});
|
||||
var current_account: ?[]const u8 = null;
|
||||
for (mismatches.items) |m| {
|
||||
if (current_account == null or !std.mem.eql(u8, current_account.?, m.account)) {
|
||||
current_account = m.account;
|
||||
try cli.printFg(out, color, cli.CLR_HEADER, " {s}\n", .{m.account});
|
||||
}
|
||||
var old_buf: [24]u8 = undefined;
|
||||
var new_buf: [24]u8 = undefined;
|
||||
var date_buf: [10]u8 = undefined;
|
||||
const old_str = std.fmt.bufPrint(&old_buf, "{f}", .{Money.from(m.old_price)}) catch "$?";
|
||||
const new_str = std.fmt.bufPrint(&new_buf, "{f}", .{Money.from(m.new_price)}) catch "$?";
|
||||
const date_str = std.fmt.bufPrint(&date_buf, "{f}", .{m.price_date}) catch "????-??-??";
|
||||
try out.print(" {s:<14} {s} → {s} ", .{ m.symbol, old_str, new_str });
|
||||
try cli.printFg(out, color, cli.CLR_WARNING, "price_date still {s} — bump it\n", .{date_str});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Find accounts modified in working copy (uncommitted changes)
|
||||
var working_copy_modified = std.StringHashMap(void).init(allocator);
|
||||
defer working_copy_modified.deinit();
|
||||
|
|
@ -2235,6 +2467,49 @@ test "option delta tracking in compareAccounts" {
|
|||
try std.testing.expect(found_option);
|
||||
}
|
||||
|
||||
test "compareAccounts: sub-dollar cash drift is flagged (cash matches to the penny)" {
|
||||
const allocator = std.testing.allocator;
|
||||
|
||||
// Portfolio cash $38.75; Fidelity reports $38.97 — a $0.22
|
||||
// money-market dividend accrual. It's below the $1 securities
|
||||
// tolerance, but cash carries no NAV rounding, so it must match to
|
||||
// the penny rather than be silently swallowed.
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "FDRXX", .shares = 38.75, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample 401k BL" },
|
||||
};
|
||||
const portfolio = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
|
||||
var brokerage = [_]BrokeragePosition{
|
||||
.{ .account_number = "1234", .account_name = "BrokerageLink", .symbol = "FDRXX", .description = "HELD IN MONEY MARKET", .quantity = null, .current_value = 38.97, .cost_basis = null, .is_cash = true },
|
||||
};
|
||||
|
||||
var entries = [_]analysis.AccountTaxEntry{
|
||||
.{ .account = "Sample 401k BL", .tax_type = .traditional, .institution = "fidelity", .account_number = "1234" },
|
||||
};
|
||||
const acct_map = analysis.AccountMap{ .entries = &entries, .allocator = allocator };
|
||||
|
||||
var prices = std.StringHashMap(f64).init(allocator);
|
||||
defer prices.deinit();
|
||||
|
||||
const results = try compareAccounts(allocator, portfolio, &brokerage, acct_map, "fidelity", prices, Date.fromYmd(2026, 6, 19));
|
||||
defer {
|
||||
for (results) |r| allocator.free(r.comparisons);
|
||||
allocator.free(results);
|
||||
}
|
||||
|
||||
try std.testing.expectEqual(@as(usize, 1), results.len);
|
||||
try std.testing.expect(results[0].has_discrepancies);
|
||||
|
||||
var found_cash = false;
|
||||
for (results[0].comparisons) |cmp| {
|
||||
if (cmp.is_cash) {
|
||||
found_cash = true;
|
||||
try std.testing.expectApproxEqAbs(@as(f64, 0.22), cmp.value_delta.?, 0.001);
|
||||
}
|
||||
}
|
||||
try std.testing.expect(found_cash);
|
||||
}
|
||||
|
||||
test "detectBrokerFileKind: fidelity csv" {
|
||||
const fidelity_header = "Account Number,Account Name,Symbol,Description";
|
||||
try std.testing.expectEqual(BrokerFileKind.fidelity_csv, detectBrokerFileKind(fidelity_header).?);
|
||||
|
|
@ -2379,6 +2654,184 @@ test "findModifiedAccounts: no changes" {
|
|||
try std.testing.expectEqual(@as(u32, 0), modified.count());
|
||||
}
|
||||
|
||||
// ── collectStaleManualPrices ─────────────────────────────────
|
||||
|
||||
test "collectStaleManualPrices: dated stale stock lot is flagged with account + age" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
|
||||
var stale = try collectStaleManualPrices(allocator, pf, as_of, 3);
|
||||
defer stale.deinit(allocator);
|
||||
|
||||
try std.testing.expectEqual(@as(usize, 1), stale.items.len);
|
||||
try std.testing.expectEqualStrings("Sample 529", stale.items[0].account);
|
||||
try std.testing.expectEqualStrings("F529A", stale.items[0].symbol);
|
||||
try std.testing.expectEqual(@as(?i32, 31), stale.items[0].age_days);
|
||||
}
|
||||
|
||||
test "collectStaleManualPrices: price within threshold is skipped" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2026, 5, 30) },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
var stale = try collectStaleManualPrices(allocator, pf, as_of, 3);
|
||||
defer stale.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), stale.items.len);
|
||||
}
|
||||
|
||||
test "collectStaleManualPrices: undated manual price is always flagged" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = null },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
var stale = try collectStaleManualPrices(allocator, pf, as_of, 3);
|
||||
defer stale.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 1), stale.items.len);
|
||||
try std.testing.expectEqual(@as(?Date, null), stale.items[0].price_date);
|
||||
try std.testing.expectEqual(@as(?i32, null), stale.items[0].age_days);
|
||||
}
|
||||
|
||||
test "collectStaleManualPrices: CDs and cash are excluded" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "CD123", .shares = 10000, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .account = "Sample IRA", .security_type = .cd, .price = 10000.0, .price_date = Date.fromYmd(2020, 1, 1) },
|
||||
.{ .symbol = "", .shares = 5000, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .account = "Sample IRA", .security_type = .cash, .price = 5000.0, .price_date = Date.fromYmd(2020, 1, 1) },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
var stale = try collectStaleManualPrices(allocator, pf, as_of, 3);
|
||||
defer stale.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), stale.items.len);
|
||||
}
|
||||
|
||||
test "collectStaleManualPrices: closed lot is excluded" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2020, 1, 1), .close_date = Date.fromYmd(2026, 1, 1), .close_price = 26.0 },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
var stale = try collectStaleManualPrices(allocator, pf, as_of, 3);
|
||||
defer stale.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), stale.items.len);
|
||||
}
|
||||
|
||||
test "collectStaleManualPrices: lot without a manual price is skipped" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "VTI", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 200.0, .account = "Sample Brokerage" },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
var stale = try collectStaleManualPrices(allocator, pf, as_of, 3);
|
||||
defer stale.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), stale.items.len);
|
||||
}
|
||||
|
||||
// ── findPriceDateMismatches ──────────────────────────────────
|
||||
|
||||
test "findPriceDateMismatches: price moved but date unchanged is flagged" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var head = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
var work = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 27.5, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
const cp = portfolio_mod.Portfolio{ .lots = &head, .allocator = allocator };
|
||||
const wp = portfolio_mod.Portfolio{ .lots = &work, .allocator = allocator };
|
||||
|
||||
var m = try findPriceDateMismatches(allocator, cp, wp, as_of);
|
||||
defer m.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 1), m.items.len);
|
||||
try std.testing.expectEqualStrings("F529A", m.items[0].symbol);
|
||||
try std.testing.expectEqual(@as(f64, 25.0), m.items[0].old_price);
|
||||
try std.testing.expectEqual(@as(f64, 27.5), m.items[0].new_price);
|
||||
}
|
||||
|
||||
test "findPriceDateMismatches: price moved AND date moved (back-date) is not flagged" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var head = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
var work = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 27.5, .price_date = Date.fromYmd(2026, 5, 29) },
|
||||
};
|
||||
const cp = portfolio_mod.Portfolio{ .lots = &head, .allocator = allocator };
|
||||
const wp = portfolio_mod.Portfolio{ .lots = &work, .allocator = allocator };
|
||||
var m = try findPriceDateMismatches(allocator, cp, wp, as_of);
|
||||
defer m.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), m.items.len);
|
||||
}
|
||||
|
||||
test "findPriceDateMismatches: unchanged price is not flagged" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = allocator };
|
||||
var m = try findPriceDateMismatches(allocator, pf, pf, as_of);
|
||||
defer m.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), m.items.len);
|
||||
}
|
||||
|
||||
test "findPriceDateMismatches: newly-added lot is not flagged" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var head = [_]portfolio_mod.Lot{};
|
||||
var work = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 27.5, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
const cp = portfolio_mod.Portfolio{ .lots = &head, .allocator = allocator };
|
||||
const wp = portfolio_mod.Portfolio{ .lots = &work, .allocator = allocator };
|
||||
var m = try findPriceDateMismatches(allocator, cp, wp, as_of);
|
||||
defer m.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), m.items.len);
|
||||
}
|
||||
|
||||
test "findPriceDateMismatches: undated working lot is left to the stale-price section" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var head = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 25.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
var work = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "F529A", .shares = 100, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 10.0, .account = "Sample 529", .price = 27.5, .price_date = null },
|
||||
};
|
||||
const cp = portfolio_mod.Portfolio{ .lots = &head, .allocator = allocator };
|
||||
const wp = portfolio_mod.Portfolio{ .lots = &work, .allocator = allocator };
|
||||
var m = try findPriceDateMismatches(allocator, cp, wp, as_of);
|
||||
defer m.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), m.items.len);
|
||||
}
|
||||
|
||||
test "findPriceDateMismatches: CD price change is ignored" {
|
||||
const allocator = std.testing.allocator;
|
||||
const as_of = Date.fromYmd(2026, 6, 1);
|
||||
var head = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "CD123", .shares = 10000, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .account = "Sample IRA", .security_type = .cd, .price = 10000.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
var work = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "CD123", .shares = 10000, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .account = "Sample IRA", .security_type = .cd, .price = 10100.0, .price_date = Date.fromYmd(2026, 5, 1) },
|
||||
};
|
||||
const cp = portfolio_mod.Portfolio{ .lots = &head, .allocator = allocator };
|
||||
const wp = portfolio_mod.Portfolio{ .lots = &work, .allocator = allocator };
|
||||
var m = try findPriceDateMismatches(allocator, cp, wp, as_of);
|
||||
defer m.deinit(allocator);
|
||||
try std.testing.expectEqual(@as(usize, 0), m.items.len);
|
||||
}
|
||||
|
||||
test "hasAccountDiscrepancies" {
|
||||
const clean = [_]AccountComparison{.{
|
||||
.account_name = "Acct",
|
||||
|
|
@ -2736,6 +3189,38 @@ test "compareSchwabSummary: cash mismatch → has_discrepancy true" {
|
|||
try std.testing.expect(results[0].has_discrepancy);
|
||||
}
|
||||
|
||||
test "compareSchwabSummary: sub-dollar cash drift is flagged (cash matches to the penny)" {
|
||||
const allocator = std.testing.allocator;
|
||||
const today = Date.fromYmd(2026, 6, 19);
|
||||
|
||||
// Portfolio cash $38.75; Schwab reports $38.97 — a $0.22 accrual.
|
||||
// Below the $1 securities tolerance, but a real cash drift that
|
||||
// must surface.
|
||||
const lots = [_]portfolio_mod.Lot{
|
||||
.{ .symbol = "CASH", .shares = 38.75, .open_date = Date.fromYmd(2024, 1, 1), .open_price = 1.0, .security_type = .cash, .account = "Sample Brokerage" },
|
||||
};
|
||||
const portfolio = portfolio_mod.Portfolio{ .lots = @constCast(&lots), .allocator = allocator };
|
||||
|
||||
const schwab_accounts = [_]SchwabAccountSummary{
|
||||
.{ .account_name = "Sample Brokerage", .account_number = "1234", .cash = 38.97, .total_value = 38.97 },
|
||||
};
|
||||
|
||||
var entries = [_]analysis.AccountTaxEntry{
|
||||
.{ .account = "Sample Brokerage", .tax_type = .taxable, .institution = "schwab", .account_number = "1234" },
|
||||
};
|
||||
const acct_map = analysis.AccountMap{ .entries = &entries, .allocator = allocator };
|
||||
|
||||
var prices = std.StringHashMap(f64).init(allocator);
|
||||
defer prices.deinit();
|
||||
|
||||
const results = try compareSchwabSummary(allocator, portfolio, &schwab_accounts, acct_map, prices, today);
|
||||
defer allocator.free(results);
|
||||
|
||||
try std.testing.expectEqual(@as(usize, 1), results.len);
|
||||
try std.testing.expectApproxEqAbs(@as(f64, 0.22), results[0].cash_delta.?, 0.001);
|
||||
try std.testing.expect(results[0].has_discrepancy);
|
||||
}
|
||||
|
||||
test "compareSchwabSummary: account_number with no match → empty account_name" {
|
||||
const allocator = std.testing.allocator;
|
||||
const today = Date.fromYmd(2026, 5, 8);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue