add stale harvesting date checks to audit command

docs/guides/set-up-accounts.md

@@ -124,9 +124,21 @@ and the TUI Portfolio tab header when you filter to that account:
 
 The date is required -- an undated hand-copied number is
 indistinguishable from a stale one, so zfin won't display it and
-[`zfin doctor`](../reference/cli/doctor.md) will tell you. Entries more
-than 12 months old stop displaying on purpose. The sign is ignored, so
-`-45300` works too.
+[`zfin doctor`](../reference/cli/doctor.md) will tell you. The sign is
+ignored, so `-45300` works too.
+
+You don't have to remember to refresh it. A flagless
+[`zfin audit`](../reference/cli/audit.md) lists anything more than 90
+days old, so it rides along with your normal maintenance sweep:
+
+```
+  Stale harvested figures (>90 days - refresh 'harvested' in accounts.srf)
+    Sample Tax Loss                  last updated 146 days ago
+```
+
+Past 12 months the annotation retires itself -- data that old isn't
+decision-useful. The audit nag keeps going and says
+`no longer displayed`, so the figure never disappears silently.
 
 **Don't put the number in the account name.** It's tempting -- the name
 already prints in a lot of places -- but the name is a join key, not a

docs/reference/cli/audit.md

@@ -8,10 +8,11 @@ Usage: zfin audit [opts]
 ```
 
 **Flagless** runs the hygiene check -- stale manual prices,
-accounts overdue for update, unhandled stock splits, and
-auto-discovered brokerage-file candidates. **With brokerage flags**, it
-reconciles your portfolio against the export (treating the brokerage as
-source of truth) and reports discrepancies.
+accounts overdue for update, stale tax-loss-harvested figures,
+unhandled stock splits, and auto-discovered brokerage-file candidates.
+**With brokerage flags**, it reconciles your portfolio against the
+export (treating the brokerage as source of truth) and reports
+discrepancies.
 
 ## Options
 
@@ -41,6 +42,14 @@ purchase date but haven't opted into automatic split adjustment, in an
 date on its `metadata.srf` row -- without it, that holding's shares (and
 every value derived from them) are misstated across the split.
 
+Accounts carrying a hand-declared
+[`harvested`](../config/accounts-srf.md#harvested-and-harvested_date)
+figure get a **Stale harvested figures** section once the figure is more
+than 90 days old, or if it has no `harvested_date` at all. The section
+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.
+
 ## Example (hygiene check)
 
 ```bash

docs/reference/config/accounts-srf.md

@@ -120,10 +120,21 @@ It appears in four places:
   figure with no date is indistinguishable from a stale one, so zfin
   refuses to show it. [`zfin doctor`](../cli/doctor.md) warns when
   `harvested` is set without a date, and when a date is in the future.
+- **`zfin audit` nags after 90 days.** A flagless
+  [`zfin audit`](../cli/audit.md) lists any account whose figure is more
+  than 90 days old under **Stale harvested figures**, so it's part of
+  your normal maintenance sweep. The threshold isn't configurable, and
+  doesn't need to be: only accounts that declare `harvested` are ever
+  considered, so not wanting the nag and not wanting the field are the
+  same choice.
 - **Entries older than 12 months stop displaying.** Harvest data that
   old isn't decision-useful. This is designed behavior, not an error --
   `doctor` stays quiet about it. Refresh the figure and the date to
   bring it back.
+
+  The audit nag keeps firing past 12 months, and says
+  `no longer displayed` once the annotation has retired -- otherwise the
+  annotation would simply vanish with nothing to explain why.
 - **The sign doesn't matter.** `harvested:num:45300` and
   `harvested:num:-45300` are equivalent; the annotation's parentheses
   already carry the accounting "this is a loss" convention.
@@ -131,6 +142,9 @@ It appears in four places:
   breakdown value, or the contributions attribution. It is an
   annotation, not an input.
 
+The two thresholds together: fresh for 90 days, nagged from 90 to 365,
+nagged *and* hidden past 365.
+
 ### Why not put it in the account name?
 
 Because the account name is a **join key**, not a label. It is matched

src/commands/audit/hygiene.zig

@@ -3,10 +3,10 @@
 //! When `zfin audit` runs with no brokerage flag, this is what
 //! executes: stale-manual-price detection, working-tree-vs-HEAD
 //! price/date mismatch detection, account-cadence overdue checks,
-//! automatic brokerage-file discovery + reconciliation, and the
-//! large-new-lot transfer nudge.
+//! stale hand-declared `harvested` figures, automatic brokerage-file
+//! discovery + reconciliation, and the large-new-lot transfer nudge.
 //!
-//! The reconciliation in Section 4 delegates to the per-broker
+//! The reconciliation in Section 5 delegates to the per-broker
 //! modules (`fidelity.zig`, `schwab.zig`) and the shared display in
 //! `common.zig`; this file owns only the hygiene logic, so a future
 //! `zfin doctor` can reuse it without pulling in the reconciler
@@ -28,6 +28,7 @@ const test_git = @import("../../testutil/git.zig");
 const common = @import("common.zig");
 const fidelity = @import("fidelity.zig");
 const schwab = @import("schwab.zig");
+const fmt = cli.fmt;
 
 // ── Hygiene check (flagless audit) ──────────────────────────
 
@@ -43,6 +44,20 @@ const stale_warning_multiplier: u32 = 2; // yellow -> red at 2× threshold
 /// pathological case where an account never changed in tracked history
 /// (so it never resolves) from forcing a scan of the entire repo.
 const max_history_commits_scanned: usize = 500;
+/// Age at which `zfin audit` starts nagging that an account's
+/// hand-declared `harvested` figure (see `AccountTaxEntry.harvested`)
+/// needs a refresh.
+///
+/// Not configurable, and deliberately so: unlike `update_cadence` -
+/// which nags every account by default and therefore *needs* a
+/// per-account silence knob - this section only ever considers accounts
+/// that explicitly declare `harvested`. Not wanting the nag and not
+/// wanting the field are the same choice, so the opt-out already exists.
+///
+/// 90 days is the same interval as `UpdateCadence.quarterly`, but is
+/// deliberately an independent constant: retuning the account
+/// reconciliation cadence shouldn't silently move this.
+const harvested_stale_days: u32 = 90;
 
 /// Type of a discovered brokerage file.
 const BrokerFileKind = enum {
@@ -421,6 +436,98 @@ 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 {
+    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.
+    age_days: ?i32,
+    /// True when the figure no longer renders anywhere.
+    ///
+    /// 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,
+};
+
+/// Collect accounts whose `harvested` figure is older than
+/// `harvested_stale_days`, or that declare `harvested` with no
+/// `harvested_date`, for the "Stale harvested figures" hygiene section.
+///
+/// Only accounts that explicitly declare `harvested` are considered -
+/// that declaration IS the opt-in to being nagged, which is why the
+/// threshold needs no per-account override.
+///
+/// Future-dated entries fall out as fresh (their age is negative).
+/// That's deliberate: `zfin doctor` already reports a future
+/// `harvested_date` as the config typo it is, and surfacing the same
+/// mistake a second time in staleness clothing would be worse than
+/// reporting it once.
+///
+/// Caller owns the returned list; `account` fields borrow from
+/// `account_map`.
+fn collectStaleHarvested(
+    allocator: std.mem.Allocator,
+    account_map: analysis.AccountMap,
+    as_of: Date,
+) !std.ArrayList(StaleHarvest) {
+    var out = std.ArrayList(StaleHarvest).empty;
+    errdefer out.deinit(allocator);
+
+    const threshold: i32 = @intCast(harvested_stale_days);
+    for (account_map.entries) |e| {
+        if (e.harvested == null) continue;
+
+        // 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;
+
+        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 });
+        } else {
+            try out.append(allocator, .{ .account = e.account, .age_days = null, .hidden = hidden });
+        }
+    }
+    return out;
+}
+
+/// Sort stale harvested figures worst-first: undated entries (which
+/// never render), 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
+/// is strictly more useful.
+fn harvestLessThan(_: void, a: StaleHarvest, b: StaleHarvest) 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| {
+        const bd = b.age_days.?;
+        if (ad != bd) return ad > bd;
+    }
+    return std.mem.order(u8, a.account, b.account) == .lt;
+}
+
+/// How many accounts declare a `harvested` figure at all. Lets the
+/// display distinguish "nothing to report because you don't use this
+/// feature" (print nothing) from "nothing to report because everything
+/// is current" (print a reassuring `(none)`).
+fn countHarvestedAccounts(account_map: analysis.AccountMap) usize {
+    var n: usize = 0;
+    for (account_map.entries) |e| {
+        if (e.harvested != null) n += 1;
+    }
+    return n;
+}
+
 /// 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-
@@ -840,7 +947,48 @@ pub fn runHygieneCheck(
         }
     }
 
-    // ── Section 3: Discover brokerage files ──
+    // ── Section 3: Stale harvested figures ──
+    //
+    // Accounts whose hand-declared `harvested` figure (accounts.srf) is
+    // older than `harvested_stale_days`, or that declare it with no
+    // `harvested_date`. Unlike Section 2 this ages an explicit date
+    // field rather than walking git history, so the two are separate
+    // sections - one "days ago" column with two meanings would mislead.
+    //
+    // The nag has no upper bound on purpose. Past 12 months the
+    // annotation stops rendering (see `format.fmtHarvestAnnotation`),
+    // and this section is then the only thing that explains where it
+    // went - so it escalates its wording instead of going quiet.
+    {
+        const declared = countHarvestedAccounts(account_map);
+        // Silent for portfolios that don't use the feature at all; once
+        // an account declares a figure, say so either way.
+        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);
+
+            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});
+
+            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 4: Discover brokerage files ──
 
     // Resolve audit directories
     const portfolio_dir = std.fs.path.dirnamePosix(portfolio_path) orelse ".";
@@ -883,7 +1031,7 @@ pub fn runHygieneCheck(
         }
     }
 
-    // ── Section 4: Auto-reconcile discovered files ──
+    // ── Section 5: Auto-reconcile discovered files ──
 
     if (all_files.items.len > 0) {
         // Build prices map (shared by all reconciliations)
@@ -1022,7 +1170,7 @@ pub fn runHygieneCheck(
         }
     }
 
-    // ── Section 5: Large new lots - confirm source ──
+    // ── Section 6: Large new lots - confirm source ──
     //
     // Cross-check any new_* Change with value >= threshold against
     // `transaction_log.srf` (via the shared contributions pipeline).
@@ -1048,7 +1196,7 @@ pub fn runHygieneCheck(
         }
     }
 
-    // ── Section 6: Unhandled stock splits ──
+    // ── Section 7: Unhandled stock splits ──
     //
     // Held symbols with a split AFTER a lot's purchase date that haven't
     // opted into automatic split adjustment. Each needs a
@@ -1373,6 +1521,187 @@ test "collectStaleManualPrices: lot without a manual price is skipped" {
     try std.testing.expectEqual(@as(usize, 0), stale.items.len);
 }
 
+// ── collectStaleHarvested ────────────────────────────────────
+
+/// Build an in-memory AccountMap from a literal SRF string, mirroring
+/// the helper in `analytics/analysis.zig`. Exercises the real parse
+/// path so these tests can't drift from how `accounts.srf` is read.
+fn testAccountMap(comptime data: []const u8) !analysis.AccountMap {
+    return analysis.parseAccountsFile(std.testing.allocator, data);
+}
+
+test "collectStaleHarvested: no account declares harvested -> empty" {
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Brokerage,tax_type::taxable
+    );
+    defer am.deinit();
+
+    var stale = try collectStaleHarvested(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), countHarvestedAccounts(am));
+}
+
+test "collectStaleHarvested: figure within the window is fresh" {
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2026-06-24
+    );
+    defer am.deinit();
+
+    // 31 days old.
+    var stale = try collectStaleHarvested(allocator, am, Date.fromYmd(2026, 7, 25));
+    defer stale.deinit(allocator);
+    try std.testing.expectEqual(@as(usize, 0), stale.items.len);
+    // ...but it IS declared, so the section still prints "(none)".
+    try std.testing.expectEqual(@as(usize, 1), countHarvestedAccounts(am));
+}
+
+test "collectStaleHarvested: exactly at the threshold is still fresh" {
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2026-01-01
+    );
+    defer am.deinit();
+
+    const on = Date.fromYmd(2026, 1, 1);
+    const at_threshold = on.addDays(@intCast(harvested_stale_days));
+    var stale = try collectStaleHarvested(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 collectStaleHarvested(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(harvested_stale_days + 1)), stale2.items[0].age_days);
+    try std.testing.expect(!stale2.items[0].hidden);
+}
+
+test "collectStaleHarvested: undated figure is flagged with a null age" {
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300
+    );
+    defer am.deinit();
+
+    var stale = try collectStaleHarvested(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 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);
+}
+
+test "collectStaleHarvested: future-dated figure is left to doctor" {
+    // A future `harvested_date` is a config typo, and `zfin doctor`
+    // already reports it as one. Surfacing the same mistake here in
+    // staleness clothing would double-nag for a single error.
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2027-06-24
+    );
+    defer am.deinit();
+
+    var stale = try collectStaleHarvested(allocator, am, Date.fromYmd(2026, 7, 25));
+    defer stale.deinit(allocator);
+    try std.testing.expectEqual(@as(usize, 0), stale.items.len);
+}
+
+test "collectStaleHarvested: hidden 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
+    // 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.
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2024-02-29
+    );
+    defer am.deinit();
+
+    const on = Date.fromYmd(2024, 2, 29);
+    var offset: i32 = 300;
+    while (offset <= 430) : (offset += 1) {
+        const as_of = on.addDays(offset);
+        var stale = try collectStaleHarvested(allocator, am, as_of);
+        defer stale.deinit(allocator);
+        try std.testing.expectEqual(@as(usize, 1), stale.items.len); // always past 90d here
+
+        // 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);
+    }
+}
+
+test "collectStaleHarvested: hidden is set once the annotation retires" {
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Tax Loss,tax_type::taxable,harvested:num:45300,harvested_date::2024-06-24
+    );
+    defer am.deinit();
+
+    // ~400 days later: stale AND no longer rendered.
+    var stale = try collectStaleHarvested(allocator, am, Date.fromYmd(2025, 7, 29));
+    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);
+}
+
+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 };
+
+    // Undated outranks any dated entry, however old.
+    try std.testing.expect(harvestLessThan({}, undated_b, old));
+    try std.testing.expect(!harvestLessThan({}, old, undated_b));
+    // Among dated, older first.
+    try std.testing.expect(harvestLessThan({}, old, newer));
+    try std.testing.expect(!harvestLessThan({}, 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));
+}
+
+test "collectStaleHarvested + sort: mixed accounts come out worst-first" {
+    const allocator = std.testing.allocator;
+    var am = try testAccountMap(
+        \\#!srfv1
+        \\account::Sample Fresh,tax_type::taxable,harvested:num:1000,harvested_date::2026-07-01
+        \\account::Sample Mild,tax_type::taxable,harvested:num:2000,harvested_date::2026-01-01
+        \\account::Sample Ancient,tax_type::taxable,harvested:num:3000,harvested_date::2024-01-01
+        \\account::Sample Undated,tax_type::taxable,harvested:num:4000
+        \\account::Sample None,tax_type::taxable
+    );
+    defer am.deinit();
+
+    var stale = try collectStaleHarvested(allocator, am, Date.fromYmd(2026, 7, 25));
+    defer stale.deinit(allocator);
+    std.mem.sort(StaleHarvest, stale.items, {}, harvestLessThan);
+
+    try std.testing.expectEqual(@as(usize, 4), countHarvestedAccounts(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);
+    // Only the ancient one has retired from display.
+    try std.testing.expect(stale.items[1].hidden);
+    try std.testing.expect(!stale.items[2].hidden);
+}
+
 // ── findPriceDateMismatches ──────────────────────────────────
 
 test "findPriceDateMismatches: price moved but date unchanged is flagged" {
@@ -1719,7 +2048,7 @@ test "findPriceDateMismatches: duplicate HEAD identity collapses to one entry" {
     try std.testing.expectApproxEqAbs(@as(f64, 160), mismatches.items[0].new_price, 0.01);
 }
 
-test "runHygieneCheck: Section 6 flags an un-opted-in symbol's split, not an opted-in one" {
+test "runHygieneCheck: Section 7 flags an un-opted-in symbol's split, not an opted-in one" {
     const allocator = std.testing.allocator;
     const io = std.testing.io;
     var tmp = std.testing.tmpDir(.{});
@@ -1773,7 +2102,7 @@ test "runHygieneCheck: Section 6 flags an un-opted-in symbol's split, not an opt
     const output = aw.written();
     try std.testing.expect(std.mem.indexOf(u8, output, "Portfolio hygiene") != null);
 
-    // Assert against Section 6 specifically (it's the last section, so
+    // Assert against Section 7 specifically (it's the last section, so
     // slice from its header to end) - robust even if NVDA/AMZN surface
     // in an earlier section. The un-opted-in NVDA is listed; the
     // opted-in AMZN is not.